[19:57:39] Reedy: how do you get a 32-bit PHP nowadays? (context: CommonPasswords change from .json to .php) [20:13:40] https://hub.docker.com/r/i386/php/ [20:13:50] Krinkle: ^ [20:44:39] Krinkle: a 32bit vm? :) [20:55:35] hm [20:55:39] or we can do it without a bloomfilter [20:56:08] dataset isn't all that big [20:57:48] SecureLinkFixer is about 3X larger and still gets away with an array [20:58:04] https://gerrit.wikimedia.org/r/c/mediawiki/libs/CommonPasswords/+/882235/ [20:59:29] it's actually faster for bloomfilter to initialise the data (if the array is warm in php-opcache instead of json) than for cold PHP to parse the .php. [20:59:33] but warm php wins either way [21:04:06] Looks likek that's the only thing in prod using that dependency, so we can axe bloom-filter [21:13:24] things change/improve/get worse over time :) [21:23:32] And especially for code that hasn't been really touched in 5? 5.5 years? [21:28:35] Yeah, for the most part, opcache and soon opcache-jit are going to change many games and invert certain optimisations [21:30:41] it's all within reason though, if we have 100 different lookup features like this, having them perform worse but in low ~1ms constant time is also benefitial given it spares opcache/RAM space increase, but for a handful like these, should be fine. [21:30:55] but for now, we can take the 0.01ms benefit :) [21:31:04] that is, going to from 1ms to 0.01ms [22:06:01] PHP Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 8388648 bytes) in /Users/reedy/PhpstormProjects/CommonPasswords/src/common.php on line 873817 [22:06:09] Of course, it does also increase memory usage... :D [22:06:43] (that's my rebasing my 1M password patch) [22:06:55] right, that's quite a bit larger [22:07:15] 1MB -> ~10MB I'm guessing [22:07:19] I forgot about that [22:07:20] yeah [22:07:29] Maybe we keep the bloom-filter for that one then... [22:08:44] it's actually more like a 16MB php array [22:09:24] Which is markedly larger than domains.php [22:12:42] I didn't create a task, but I think the intention was after seeing that best practices was suggesting a list of ~1M common passwords be used for exclusion