On Wed, 26 Feb 2025 13:00:51 GMT, Roberto Castañeda Lozano <rcastaned...@openjdk.org> wrote:
>>> > @robcasloz I identified and hopefully fixed a small issue that hit the >>> > "disabled" path. Turns out we allocate arena chunks a lot more frequently >>> > than I thought, and the new unconditional call to Thread::current() in >>> > there was hurting a bit. I now avoid this unless I know the statistic is >>> > enabled. >>> > With this patch, on my machine the difference between unpatched and >>> > patched JVM with stats disabled is below one standard deviation for the >>> > benchmark in question. >>> >>> Great, thanks! Will re-run benchmarking and report results early next week. >> >> Functional test results (Oracle tier1-5) still look good for the latest >> commit (dd7a06ad). I can confirm that the C2 speed regression on our >> linux-x64 machines is almost fully mitigated. The 2-3% regression on our >> macosx-aarch64 machines does not seem to be addressed by the latest changes >> though, but as I mentioned before I think it is in the acceptable range (and >> only affects one benchmark). > >> @robcasloz, @ashu-mehra thanks a lot for your reviews. I incorporated most >> of them into the PR. > > Thanks, Thomas! I see that the changes suggested in > https://github.com/openjdk/jdk/commit/d501bd8a674229904358fb168a9c347004efeea3 > are not incorporated, is it because you find them out of the scope of this > PR? I would argue that at least tagging `Compile::_Compile_types` with > `tag_type` is relevant and in line with the other changes included in this > PR, e.g. [this > one](https://github.com/openjdk/jdk/pull/23530/files#diff-3559dcf23b719805be5fd06fd5c1851dbd8f53e47afe6d99cba13a3de0ebc6b2R443). > > @robcasloz, @ashu-mehra thanks a lot for your reviews. I incorporated most > > of them into the PR. > > Thanks, Thomas! I see that the changes suggested in > [d501bd8](https://github.com/openjdk/jdk/commit/d501bd8a674229904358fb168a9c347004efeea3) > are not incorporated, is it because you find them out of the scope of this > PR? I would argue that at least tagging `Compile::_Compile_types` with > `tag_type` is relevant and in line with the other changes included in this > PR, e.g. [this > one](https://github.com/openjdk/jdk/pull/23530/files#diff-3559dcf23b719805be5fd06fd5c1851dbd8f53e47afe6d99cba13a3de0ebc6b2R443). Sorry, this was an oversight. Will do this, no reason not to. ------------- PR Comment: https://git.openjdk.org/jdk/pull/23530#issuecomment-2685412110
