Page MenuHomePhabricator

Some lua-calls with language specified does not end up in formatterCache
Closed, ResolvedPublic8 Estimated Story Points

Description

Issue

While looking into some formatterCache things in T252595 I noticed this.

Creating a module like such

local p = {} --p stands for package

function p.hello( frame )
    return mw.wikibase.getLabelByLang('Q28', 'en')
end

return p

And using it on a page like this on a client

{{#invoke:LuaTest|hello}}

These values fetched never end up in or look at the formatterCache, which they probably should.

Using methods without the language we start using the cache.

return mw.wikibase.getLabel('Q28')

There seems to be a mismatch between the caching policies of newLanguageIndependentLuaBindings / newLanguageDependentLuaBindings (both methods of Scribunto_LuaWikibaseLibrary).

Acceptance Criteria

  • Caching policies between newLanguageIndependentLuaBindings and newLanguageDependentLuaBindings match (using caching in both when currently used i either one)

Notes

  • mw.wikibase.getLabel does consider "language fallback"
  • mw.wikibase.getLabelByLangdoes only get the label in the language request, not performing any "fallback" logic
  • Conclusion of the above: those method should use distinct cache keys
  • Environment to reproduce the behaviour under inspection: Scribunto extension must be enabled, repo-client pair needs to be set up. Client wiki would get a Lua module with the function calls as shown in the examples above.

Reading
https://doc.wikimedia.org/Wikibase/master/php/md_docs_storage_terms-caching.html

Event Timeline

toan renamed this task from Language dependent lua-calls does not end up in formatterCache to Some lua-calls with language specified does not end up in formatterCache.Sep 28 2020, 2:17 PM
toan added a project: Wikidata-Campsite.
  • mw.wikibase.getLabel does consider "language fallback"
  • mw.wikibase.getLabelByLang does only get the label in the language request, not performing any "fallback" logic
  • Conclusion of the above: those method should use distinct cache keys

I think we actually can reuse the same cache, as long as the cache values indicate whether they used language fallbacks (⇒ getLabelByLang should return nil) or not (⇒ getLabelByLang can return the same label). But maybe a separate cache would be simpler.

WMDE-leszek set the point value for this task to 8.EditedOct 21 2020, 1:47 PM
WMDE-leszek added subscribers: Tarrow, Addshore, WMDE-leszek.

@Addshore: The team will work on it. However, when discussing the task, @Tarrow brought the question on whether we know what is the expected performance impact of this change, or do we just anticipate there will be some but not know what level of?

Task inspection notes:

  • newLanguageDependentLuaBindings() creates a CachingFallbackLabelDescriptionLookup to take care of the caching. newLanguageIndependentLuaBindings() doesn’t create a caching version of the TermLookup it injects into the WikibaseLanguageIndependentLuaBindings. However, a CachingPrefetchingTermLookup class does exist – we can hopefully use it here without too much trouble. ⇒ T266144
  • Both CachingFallbackLabelDescriptionLookup and CachingPrefetchingTermLookup use the TermCacheKeyBuilder trait to manage cache keys. We want to look further into this ⇒ T266141.
  • We’re unhappy with the current documentation/names of some related interfaces ⇒ T266145.
WMDE-leszek claimed this task.

Development work done. Performance impact unknown and not checked.