hhvm
hhvm copied to clipboard
facebook
[AArch64] Adjust cache location for PC relative branches
AArch64 supports up to +/-128MB PC relative branches with 26-bit immediates in the B and BL instructions, so make Hot, Prof, Main, and Cold caches 128MB (total).
No new failures in the 'all' unit test suite, nor in the oss- performance benchmarking suite. When combined with relocation, it produces nice gains in oss.
@mxw please review. @swalk-cavium This patch goes well with relocation, #7661. Care to test it?
@jim-saxman - Is this branch complete? Or do I have to do the cherry-picking business on top of #7661.
@swalk-cavium It has no dependencies against #7661 so it will apply cleanly either way. However you won't see any gains if you don't also have #7661 in your tree.
We definitely can't just quit if the code size is too big—some applications might need all that code.
@jim-saxman - I think I merged #7661 and this change correctly and ran a MOP with 6 option sets. No new regressions were detected. I ran it on a Ubuntu 16.04 machine.
HI @mxw I've updated this PR by moving the Hot cache to between Main and Cold.
However, Dave and I still have a concern. Why are we running code from Cold often enough that this really makes such a difference? Specifically, is something being incorrectly placed in Cold with the hint "Unlikely" ?
I used #7703 from @swalk-cavium to dump the caches during the OSS/Mediawiki benchmark, and under 3% of the total cycles are spent in Cold). I then filtered by the addresses of the cold cache using the -a and -A flags, and the top hitter in Cold is part of is the Hooks::run php function (from the /tmp/hhvm-nginxxUDocX/mediawiki-1.28.0/includes/Hooks.php file line 131). I think, but I'm not sure, that the foreach on line 132 is in Hot, as is the return true on line 207, however the foreach's body from lines 133 to 205 is placed into Cold. This is true on ARM and x64. Why? I was expecting to find an exception handler or something else "unlikely", but finding the body of a foreach loop confuses me. Any light you can shine would be greatly appreciated!
@mxw has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
@mxw, Hi. What's the status on this patch? Would you like it changed in some fashion? I just re-benchmarked it last night and it still provides solid gains across OSS, especially Mediawiki and Drupal7.
Hi @mxw Do I have a chance of getting this PR accepted? Are there any changes you'd like me to make?