Reduce memory footprint of PhpStormStubsSourceStubber and MemoizingParser

[kukulich]

Fixes #624

[ondrejmirtes]

Still more than double than before I guess :( @Ocramius is right that this should be supported but if I had to choose between:

1. Be correct for a small percentage of users that would take advantage of case-insensitive constants and make the library worse for everyone
2. Be a little bit incorrect (that we were this way until 4.3) and consume much less memory

...it's obvious what I'd choose :)

[kukulich]

The rest of the memory usage is probably not related to case insensitive constants.

PR builds: https://github.com/Roave/BetterReflection/actions?query=branch%3Aphpstorm-stubber
The last one is without static properties and the memory usage changed from 216.00 MB to 1.41 GB.

Current PR uses static properties again and it decreased from 2.31 GB to 1.38 GB: https://github.com/Roave/BetterReflection/pull/627/checks?check_run_id=718176772

I've tried revert of case insensitive constants just to test it and it's even worse - 1.85 GB: https://github.com/Roave/BetterReflection/pull/628/checks?check_run_id=718188887

[kukulich]

1.41 GB - Already contains case insensitive constants.
https://github.com/Roave/BetterReflection/runs/717048810?check_suite_focus=true

After #606 has been merged: 2.55 GB
https://github.com/Roave/BetterReflection/runs/717069119?check_suite_focus=true

So it looks it's something in #606.

[Ocramius]

Can you try reverting my changes to the memoizing source locator? Maybe that had a bigger impact than expected.

…

On Thu, May 28, 2020, 22:41 Jaroslav Hanslík ***@***.***> wrote: 1.41 GB - Already contains case insensitive constants. https://github.com/Roave/BetterReflection/runs/717048810?check_suite_focus=true After #606 <#606> has been merged: 2.55 GB https://github.com/Roave/BetterReflection/runs/717069119?check_suite_focus=true So it looks it's something in #606 <#606>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#627 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABFVECCLPT63FUDAPZURHLRT3D75ANCNFSM4NNLKA7A> .

[kukulich]

@Ocramius Tested just now. It decresed to 178.00 MB (this PR + the revert). Probably problems with serialize/unserialize.

[kukulich]

It also faster: 12 seconds vs 18 seconds on my ntb

[Ocramius]

Yeh, strings are heavy. We can run with the revert, but I'll need to also ensure that a few of the tests run in a separate process.

Also unsure about reverting the constraint that guarantees distinct AST nodes...

[ondrejmirtes]

BTW what we did in CachedParser in PHPStan (same concept as Memoizing) some time ago is that we don't keep all cached keys in memory at once. There's an upper limit and keys get evicted when newer ones appear. Surprisingly this has minimal performance impact, but memory footprint is smaller. See: https://github.com/phpstan/phpstan-src/blob/master/src/Parser/CachedParser.php

[ondrejmirtes]

Funny realization for me - there's zero memory impact if I use $sourceCode or hash('sha256', $sourceCode) as array keys as there's probably some hashing function on the PHP side applied anyway.

[kukulich]

I've improved the memory usage with json_encode instead of serialize. It's not perfect, it's just better.

[kukulich]

The solution here is not optimal (it's a little ugly) but has better memory usage than serialize/unserialize

[Ocramius]

LGTM 👍

[Ocramius]

Thanks @kukulich!