Page MenuHomePhabricator

Scribunto function to return current expensive function count
Open, Needs TriagePublic


Scribunto currently has the function mw.incrementExpensiveFunctionCount, which I gather is supposed to be called by a module function that is considered expensive. (I haven't seen it used that way yet.)

On English Wiktionary, this function is sometimes used to detect whether the expensive function limit has been reached, and thus (ostensibly) prevent the "too many expensive function calls" error message from being triggered:

if pcall(mw.incrementExpensiveFunctionCount) then
  -- code block in which an expensive function is called

This is clumsy and, I just realized, doesn't really work: when the expensive function count is one less than the limit (499 on English Wiktionary), calling mw.incrementExpensiveFunctionCount brings it to the limit (500), and then using another expensive function in the if-block increments it past the limit (to 501) and triggers the error message. So there is probably going to be at least one error on a page that goes over the expensive function limit solely through the code block shown above.

And of course because the code includes two parser functions, the limit will be reached twice as fast.

As a replacement, it would be useful to have a function be added that returns the current expensive function count, perhaps named mw.getExpensiveFunctionCount. The code above could then be replaced with the following, which would actually reliably work.

if mw.getExpensiveFunctionCount() <= 500 - n then
  -- code block in which as many as n expensive functions might end up being be called

Without such a function, the way to prevent the module errors is to create a list of the pages that are triggering the expensive function error and exclude them manually from running the portions of code that contain the expensive parser functions. If the pages later on end up running below the limit for whatever reason, there isn't a simple way to detect this and update the list.

I'm curious if this would be possible or desirable, or if there are reasons why a function for incrementing the expensive function count has been created but one for returning the count has not.

Event Timeline

This would probably be contrary to T67258: Information can be passed between #invoke's (tracking). True, the fact of having expensive functions does that too to an extent, but there's not much we can do about that if we want to have a limit at all.

I also note that the Parser class doesn't give a sane way to access the current count (although it can be done by directly accessing the field that is public for hysterical raisins). Ideally an accessor would be added if this were to be done.

Even with T67258, I think code should be able to ask "Am I at or over the limit right now, yes or no."

Yes, this would allow "leaking" but it's "half the price" of the alternative:

result1 = do expensive parser function

result2 = do dummy/throwaway expensive parser function that should give result A if it succeeds and result B if it fails

if result2 is A then result1 is OK else result1 may or may not be OK: result1 = call alternative code

In other words, NOT having this test encourages inefficient behavior.

As a particular use case, I would like to rewrite as a module. It would be very helpful to test if I am over the limit, and if I am, then add the links to AFD1, AFD2, AFD3, AFD4, and MFD which are normally hidden if those discussions do not exist. Yes, they would be red. The current behavior of the template is to hide them if they do not exist OR if the expensive parser function count is exceeded, since the results of #ifexist are the same either way.

This is already possible:

function expensiveParserFunction(page)
	local success, result = pcall(function()

	if success then
		return 'parser function count not reached'

	return 'parser function count reached'

Sure, that would still include the page into the tracking category (see the page with the similar module code), but that probably happens with the current example, too. I’m not sure why someone would need the parser function count to do what you are describing.