Page MenuHomePhabricator

Checking existence of files with both title.exists and title.file.exists increments the expensive function count by two
Open, LowPublic

Description

With gerrit 178698, Scribunto's mw.title objects no longer increment the expensive parser function count upon creation, instead incrementing it when "expensive" properties such as title.exists are accessed.

While playing around with this new functionality, I found that for files, existence checks using title.exists and title.file.exists increment the expensive function count separately.

For example:

local title = mw.title.new( 'File:Example.png' )
local temp = title.exists
temp = title.file.exists

This code will increment the expensive function count by two, even though it's checking essentially the same thing. The same goes for checking the other way around:

local title = mw.title.new( 'File:Example.png' )
local temp = title.file.exists
temp = title.exists

That's also +2 to the expensive function count.

Ideally, both of these examples would only add one to the expensive function count, although I can't see this problem actually affecting modules in practice, as I can't imagine a reason for using both title.exists and title.file.exists on the same file.

Event Timeline

MrStradivarius assigned this task to Jackmcbarn.
MrStradivarius raised the priority of this task from to Low.
MrStradivarius updated the task description. (Show Details)
MrStradivarius added a project: Scribunto.
MrStradivarius changed Security from none to None.
MrStradivarius subscribed.

I agree we can probably make the second example cheap, but not the first one. Remember that a file description page can exist without a file existing.

Good point - I was forgetting that. It looks like the documentation needs to be changed then. At the moment it says that title.exists is an alias for title.file.exists, but the two return different results if the file description exists but the file doesn't.

Ah, the docs say the Media namespace, not the file namespace, so actually they were correct. I'll edit them a bit to make the distinction between Media namespace titles and File namespace titles clearer.

This task has been assigned to the same task owner for more than two years. Resetting task assignee due to inactivity, to decrease task cookie-licking and to get a slightly more realistic overview of plans. Please feel free to assign this task to yourself again if you still realistically work or plan to work on this task - it would be welcome!

For tips how to manage individual work in Phabricator (noisy notifications, lists of task, etc.), see https://phabricator.wikimedia.org/T228575#6237124 for available options.
(For the records, two emails were sent to assignee addresses before resetting assignees. See T228575 for more info and for potential feedback. Thanks!)

File description pages can exist without a file existing (manually created pages in the File namespace) and files can exist without description pages existing (Commons files without local description pages), so title.exists and title.file.exists are independent. I propose declining this task.

File description pages can exist without a file existing (manually created pages in the File namespace) and files can exist without description pages existing (Commons files without local description pages), so title.exists and title.file.exists are independent. I propose declining this task.

They could both be fetched in a single batch when either one is accessed, thus only needing one increment instead of two (like mw.title.newBatch​{"File:Foo", "Media:Foo"}​:lookupExistence()​:getTitles() (T376564), except with NS_MEDIA fetching fixed).

Without looking up the code, I don’t think this is feasible: a single batch requires a single source, but the page and file existence are stored differently (even when the file is local, but even more if Commons comes into play). So checking for the existence of a description page and its file is really more expensive than checking for the existence of two description pages, no matter how hard Scribunto tries to optimize it.