Skip to content

[1.15] Manually inline PooledBlockPosition#d(int, int, int)#3638

Merged
aikar merged 2 commits into
PaperMC:masterfrom
astei:improve-pooledblockposition-inlining-1.15
Jun 28, 2020
Merged

[1.15] Manually inline PooledBlockPosition#d(int, int, int)#3638
aikar merged 2 commits into
PaperMC:masterfrom
astei:improve-pooledblockposition-inlining-1.15

Conversation

@astei
Copy link
Copy Markdown
Contributor

@astei astei commented Jun 27, 2020

I was working with JITWatch, a tool that allows one to easily analyze JIT behavior and found that C1 wouldn't inline PooledBlockPosition#d(int, int, int). This is especially noticeable at -XX:MaxInlineDepth=15 (the default in JDK 14 and above). Even at the default depth of 9, this method still ranks within the top 100 of methods that the JIT couldn't easily inline.

Here's what it looks like before the change:
image

After the change:
image

@aikar aikar force-pushed the improve-pooledblockposition-inlining-1.15 branch from cb83bf3 to 1d1c056 Compare June 28, 2020 09:10
@aikar aikar merged commit 1d1c056 into PaperMC:master Jun 28, 2020
@VADemon
Copy link
Copy Markdown

VADemon commented Jul 2, 2020

Bear with me and my limited understanding as of now, but why would you care about C1 performance? Further, your first screenshot shows C1 @ Level 2 while second screen is C2 compiled code - your VM wasn't warmed up properly.

For a better understanding of compiler levels, read this excellent in-code comment: JVM8 - almost identical commment but moved since: JVM15

This is especially noticeable at -XX:MaxInlineDepth=15

One more block Lets dig deeper. The error message says "callee too large" and then the extended error message explicitly says

"The callee method is greater than the max inlining size at the C1 compiler level"

I didn't notice that description initially, but suspected that. Therefore the MaxInlineDepth has no effect: c1_GraphBuilder.cpp#L3815 is:

if (callee->code_size_for_inlining() > max_inline_size() ) INLINE_BAILOUT("callee is too large");

Q.E.D. ∎

But that's C1 only. If I understand it correctly, all of those compiler threshold cmdline settings used to apply to C1 as well, but were deprecated since. These thresholds seem to be hardcoded for C1 production builds (e.g. C1MaxInlineLevel variable set to 31-32 afair). But they continue to work for C2(?)

That's all I wanted to say from my JVM inspection tour. So:

  • Do we really need to hand-inline for intermediate C1?

If not, consider reverting the patch:

  • Confirm the method does get inlined at C2

  • If it doesn't get inlined at default settings, test with increased -XX:InlineSmallCode=n and -XX:MaxInlineSize=n

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants