[01:04:28] https://hacks.mozilla.org/2022/03/performance-tool-in-firefox-devtools-reloaded/ [02:54:01] AaronSchulz: do you have any ideas for making stashedit work with IP masking? [02:54:34] the user is logged out when the stash is saved, but logged in as a temporary user when the stash is fetched, so it's always a cache miss [02:55:04] just wondering how important it is to have the user name in the cache key [03:51:10] * AaronSchulz looks [03:54:30] TimStarling: as long as it handles edge cases of things happening between the stash and edit (user logs in with a real name, user gets renamed), it's fine [03:54:48] using the name was an easy way to deal with that [03:55:21] user_touched is too prone to be bumped for random nonsense [03:56:39] are temporary names too expensive (or logistically hard) to assign on stash? [03:56:57] is the concern that if the user logs in, the parser options may change? [03:58:08] the product idea is that the serial number should be as short as possible, which means that we should try not to assign them when an edit is not actually going to happen [03:58:57] we can determine when the edit is stashed whether temporary account creation will happen on page save, and change the key [03:59:17] I suppose you could put it that way (the UserIdentity used for ~~~~ is an option afaik) [03:59:45] yes, ok [03:59:51] that would lower the stash hit rate to some extent though [04:00:29] but I guess there is no way to know what serial ID is used if assignment is deferred [04:00:43] though, few articles involve ~~~~ [04:01:28] there is a ParserOutput flag indicating the presence of ~~~~ [04:02:53] right, though the api includes de-duplication (reuse if the text hash and other key bits match). The point is to avoid re-parsing. Getting the key should not require parsing itself. [04:03:24] of course, the client JS tries not to send text if no change event happened [04:04:18] maybe we could live without server side de-duplication [04:05:09] When you type the summary, it still has to send the text/summary to the API to prime abusefilter caches (though the client will just send a text hash if the content is the same) [04:07:19] the client gets the hash from the last case where the text had to be sent...so I guess the backend could include "user name used in parser options" in the response and the client could include that in for the summary-update-only API requests [04:08:55] I think the simplest (worst) solution apart from what it's doing already is to \have PageEditStash::parseAndCache() fail with an error if the ParserOutput contains a signature [04:09:21] though I guess the server knows the login status for all those stashedit requests anyway [04:09:58] and to share anon caches and to use the anon cache for temp user creation [04:10:10] nevermind the last bit [04:10:24] * AaronSchulz was thinking out loud [04:10:54] what it's doing already in the proposed patch is to always stash and always throw away the stash [04:13:56] it looks like the client side will always retry on error, so we could have it not do that, so the first instance of a stashed signature will make it stop stashing [04:14:01] anyway, you could assume that whether the username is used in the parser output for the given text (on edit submission) does not itself depend on the user name. So, the that could be a kind of "vary" flag stored within the stash output but not part of the key. [04:15:13] I already see similar "vary" logic in PageEditStash::checkCache [04:16:17] right, including the weird thing of constructing a User object so that you can get a WebRequest out of it [04:16:30] the common case is that content NS pages do not use ~~~~ (some WP: and meta pages might) so they will not have the penalty for logged out users who have no mask assigned yet [04:17:52] if the vary flag has the actual parser output name, then user rename races conditions are handled (we could also just ignore those to be honest) [04:19:43] though I guess ?action=purge is not always a fix if subst: is involved (but that's an edge case of an edge case) [04:20:09] you mean if the user has an edit window open with ~~~~ and they are renamed just before they hit save -- maybe it is not a requirement that they see their new name [04:20:53] well, the lag is actually based on the stash TTL...so it could be a few min [04:21:08] hmm, maybe that's why I cared enough to put that code there [04:21:58] MAX_SIGNATURE_TTL kind of mitigates MAX_CACHE_TTL [04:22:15] so the use of ~~~~ will lower the stash TTL from 5 to 1 min [04:22:29] (on top of the name being in the cache key now) [06:33:31] Krinkle: the profiler is cool, the bad thing it adds so much overhead if you enabled it for synthetic testing. For Chrome the overhead is small (at least with just a couple of trace categories) but for Firefox its at least 100%, it also makes other metrics unstable. [06:37:55] The profiler also have special handling if you collect visual metrics with Browsertime, so that you can see that in the profiler, which is cool. The bad thing is that that part is broken at the moment so I filed a bug :) [06:39:00] I had the idea before that we should run some tests with the profiler turned on, so its easy to just get the data but it was affecting the other metrics so much so I'm not sure. [06:59:13] Krinkle: There's a new post from the LinkedIn team about serving different content to different users using machine learning https://blog.tensorflow.org/2022/03/how-linkedin-personalized-performance.html and the repo https://github.com/linkedin/performance-quality-models/tree/main/ssr-mobile-web/mweb-jan-2022-v1 [07:01:52] These kind of post always include more engaged users or more revenue but never (or at least what I've seen) touches how you change your testing. [07:48:06] Krinkle: one more thing about our discussion yesterday about a mobile lab: I'm gonna prepare some tests that we can look at maybe next week (I don't have any phones with me now) and one thing that I forgot to mention is the rollout of functionality and different behaviour for example with the drift of the metrics as reported in https://bugs.chromium.org/p/chromium/issues/detail?id=1291502 [07:50:50] If I understand correctly different new functionality is rolled out differently on Android vs Desktop, meaning when we test using emulated mobile (on desktop) it not the same functionality that actually runs on Chrome on Android. Also that bug with the drift of the metrics I could only reproduce that on desktop and emulated mobile. On real mobile phones the metric aren't drifting (as far as I could test locally). [07:57:19] And I mean the Mozilla team run tests on Linux/Windows/Mac since the implementation differs, but that would be overkill for us. However having some mobile tests would be cool :) [08:58:42] phedenskog: interesting. Good to see they're engaged with it at least. [08:59:42] phedenskog: btw while we don't have platform+browser, afaik UA parser names chrome different for android so we do have that basically, and the RUM data you shared there is desktop only I think, as such.