[01:19:56] https://github.com/facebook/hhvm/issues/8317 hmm [01:20:08] ?? isn't the exact same across PHP 7 versions [02:04:11] legoktm: lol, there is a __isset magic handler? [02:04:16] That's terrible [02:04:53] because that would map to key_exists rather than isset() because isset() is key_exits + value !== null. [02:05:13] it is expected that ?? takes the second value if the former exists as null [02:41:32] Krinkle: welcome to PHP? :) [02:44:09] legoktm: so... does that mean an __isset() handler bear the responsibility to also cover __get doing null? [02:44:22] Or is the idea that the PHP engine will also invoke __get if __isset gives true? [02:44:25] to check null [02:44:45] that would be more reasonable, and just means that __isset was poorly named (e.g. should be __exists or some such) [02:44:46] I think the former [02:45:13] https://secure.php.net/manual/en/language.oop5.overloading.php#object.isset [02:45:50] Hm.. [02:46:11] Right and the "default" implementation just does exists()=true *and* get() !== null [02:46:30] and custom code can decide on their own semantics [02:46:38] oh well [02:47:10] https://codesearch.wmflabs.org/deployed/?q=__isset&i=nope&files=&repos= [02:47:19] I guess Erik ran into it in elastica [02:50:24] ha, indeed. That would make get() throw InvalidException [02:50:26] ouch [02:50:50] https://gerrit.wikimedia.org/g/mediawiki/vendor/+/8c405f8f2ff3facd980563fd20f8f7878d27ec3e/ruflin/elastica/lib/Elastica/Document.php#75 [02:51:14] isset() should never throw a fatal exception [02:51:23] but would now [10:42:25] RoanKattouw: tell me when you're around [11:53:13] TimStarling: can you help with investigating https://phabricator.wikimedia.org/T204065 ? [11:53:53] WikibaseLexeme is hitting the same problem in CI we had before: bad stuff cached in NameTableStore. [11:54:12] I tried a quick fix, but it didn't seem to work: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/459820 [11:55:11] The strange thing is that it's not the WikibaseLexeme tests that are failing. It's parser tests, when running in CI of the WikibaseLexeme repo [11:55:40] e.g. https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-php70-docker/9905/console [15:01:20] Amir1: not much this week, I'm at our team offsite [15:02:01] RoanKattouw: oh, have fun. Take a look at some gerrit patches I made when you're a little bit free [17:46:20] MatmaRex: During these two ULS commits I noticed that most of that work was diminished by the fact that it uses mw.Message#text once [17:46:20] https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/UniversalLanguageSelector/+/459011/ [17:46:29] Which apparently is quite expensive in jqueryMsg [17:46:44] I noticed that even in text() mode, it still creates DOM nodes and text nodes etc. [17:47:12] I'm thinking of maybe changing it so that in text mode, instead of appending child nodes and computing $.text(), to just concat actual strings. [17:47:38] But could use a bit of guidance and totally unfamiliar with the code. [17:47:50] I can share some flame graphs to see where the pain points are. [17:52:38] anomie: are you ok with the parser tests patch now? https://gerrit.wikimedia.org/r/c/mediawiki/core/+/459820/9 [17:52:44] Krinkle: in text() mode, it still parses things like {{GENDER:}} and {{PLURAL:}}, and i guess it does that by shuffling around DOM text nodes (even if there's no markup) [17:53:09] Krinkle: you can use plain() mode to do no parsing whatsoever, if it's a performance problem, and the message doesn't need parsing [17:53:29] MatmaRex: I change one of the messages from text() to plain() for that reason [17:53:33] and it made no difference [17:53:41] because, someone was smarter than us. [17:53:52] text() does plain() if it contains nothing that needs parsing [17:53:56] there's a... regex [17:54:12] I didn't see it at first [17:54:20] DanielK_WMDE: Yes. [17:54:31] hm [17:54:41] okay, pretty sure that plain() used to not even call the parser [17:54:46] plain() and text() both go to jqueryMsg, even plain(), and then the overide of parser() invokes the original version if it is format=plain or regex not match [17:54:49] it would just return the string from mw.messages [17:55:08] MatmaRex: yeah, but it does use parser() because that's where (by default), $N replaces happens [17:55:16] which are supported in plain() [17:55:22] it's fine though, no perf hit [17:55:49] but yes, it wasn't like that originally. [17:57:43] okay. so plain() isn't broken? good [17:59:18] Krinkle: i'm not really "familiar" with jqueryMsg, either. i just fixed some bugs in it once upon a time. but i think it really depends on building DOM nodes. so if you want to change that, it would be great, but it seems like it would be a big rewrite [18:00:32] Hm.. I was afraid of that [18:02:09] Krinkle: you could try to replace mw.jqueryMsg.HtmlEmitter with a different class that would output strings insteead of DOM nodes. but the input to the emitter (the AST) can already contain structures which can't be represented as text (like, parsed HTML syntax from the message) [18:02:54] i don't know if it's feasible [18:03:38] yeah, currently I think it's written in a way that it only puts text DOM nodes in there, but if for some reason that isn't right, it currently gracefully works by flattening it at the end with text() [18:03:40] $.text() [18:04:01] changing it would make it hard fail at the end if we just .join() and cast any non-string to string [18:04:12] and by hard I mean, [object] appearing probably [20:02:55] DanielK_WMDE: Is the plan to switch testwiki to read-new just after the train next Tuesday?