respect jit.max to stop compilation by kares · Pull Request #5870 · jruby/jruby

kares · 2019-09-10T20:13:01Z

... after some time esp. for big (Rails) apps that seemingly always execute some 'new-hot' methods

JIT parts also seemed ripe for a cleanup, so there's a few bits such as :

not having multiple callCount in a hierarchy
negated JIT condition - JITing when JIT meant to be disabled
get JIT excludes to also exclude block compilations
more useful ji logging - seeing exception.toString failures

aside from these, there's 2 simple but important changes :

-Xjit.max=10000 default, to compilations and not leak meta-space
even for a large Rails app this should be conservative as it takes several hours to reach the limit
from a production app 10_000 JIT compiles means having around 550 of allocated meta-space
Xjit.maxsize reduced from 2000 -> 1000 again based on real world production data
there's very little JIT eligible large methods in Rails, so this further helps a bit saving on meta-space

also, since I will like to add JMX setters for the jit params, they are being read from the instance config

headius

Seems like some good cleanup but I had a few questions.

core/src/main/java/org/jruby/compiler/BlockJITTask.java

headius · 2019-09-11T20:22:57Z

core/src/main/java/org/jruby/internal/runtime/AbstractIRMethod.java

-    protected int callCount = 0;
-    private MethodData methodData;
+    protected volatile int callCount = 0;
+    private transient MethodData methodData;


Is the volatile here for disabling? Since our count is always checking > threshold I never bothered to make this volatile since someone will eventually cross the line. If it's volatile it will force a volatile read for every call.

And why transient?

transient just a marker that its a cached field.
method pbly can't serialize state due the IR scope but if it did this field is not needed

core/src/main/java/org/jruby/internal/runtime/AbstractIRMethod.java

core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java

headius · 2019-09-11T20:28:14Z

core/src/main/java/org/jruby/runtime/InterpretedIRBlockBody.java

+
+                    runtime.getJITCompiler().buildThresholdReached(context, this);
+                }
+            }


This logic is very similar to that inside the method/block jit stuff, isn't it?

yy - would need a common super type if we're to keep field read write (this.callCount++)

the setter is synchronized - wanted to avoid sync cost, but it likely does not need to be if it isnt called externally.

core/src/main/java/org/jruby/runtime/MixedModeIRBlockBody.java

headius · 2019-09-11T20:34:22Z

default JIT max seems reasonable. I wonder if we should warn in some mode (maybe always, maybe only verbose mode, I don't know) that the limit has been reached? 10k methods is a lot, especially if a large Rails app is taking hours to get there.

2000 to 1000 limit seems reasonable because this is IR count and IR is much more dense than bytecode. I'll be doing some review of all IR jit logic to see what else we can shrink, also.

Live JIT flags seem good too...useful for tuning exploration etc.

remove duplicate code and share JIT-ing attempt between method impls since we do not mind delaying JIT let's not sync callCount r/w the counter will eventually reach the threshold (from a single thread) still make sure we do not submit a method twice by setting, and always checking, a low (negative) call count value

... using the same "fast" call counter algorithm

previously compilation was unbounded, JRuby will know use a threshold at which compiling stops - potentially to avoid leaking meta-space

that are within/belong to specified exclude targets

in a typical Rails app there are only a few JIT eligible methods of > 1000 instructions, thus a higher default does not make sense on a production Rails app a lower default reduced the overall generated code size by 10% wout negative performance degradation, while avg code size went from 8049 -> 7860 bytes

its fine for us to loose some counts from other threads - we always check for >= 0 - setting to a negative value while counting now uses MIN_VALUE and is synchronized

(in interpreted method/body) to disable build promotion full builds are also promoted when compilation is OFF

kares · 2019-09-12T14:34:45Z

logging for eligible JIT methods being skipped due max reached is there (did not want to stop the queue, the next thing I want to do is to allow changing JIT related config from JMX): with -Xjit.logging
https://github.com/jruby/jruby/pull/5870/files#diff-0da2daefadcea7c708815b454db3d439R184

so we can re-instantiate byte-code size check in a single place

since we'd like to know the total number of compiled classes

implementation class will now return enclosing scope module, with updated log messages we already distinguish block/method

kares · 2019-09-14T17:47:03Z

now updated with more work, parts of which I am going to need to test out JITing in production.
(namely the possibility to change jit parameters from JMX - code should be fine to handle that)

also, have tested locally - block JIT excludes work as well as reaching jit.max halts compilation

headius · 2019-09-16T19:40:12Z

I'm fine merging this any time. @enebo should probably coordinate with you because I think he was playing with some stuff that might overlap.

kares · 2019-09-16T21:13:28Z

actually do have @enebo's counter_nuke work merged on top of this (in another branch) with some fine tuning (ability to disable jit.time.delta)

enebo · 2019-09-16T21:25:19Z

@kares @headius I think merging this will be fine even it counter_nuke ends up solving the original issue as well. if aging works really well then a jit.max may never be used but I could see it as an insurance policy.

A second thought is we should consider decrementing jit count if an oneshotclassloader unloads because the method went away. I do not think that is important for this PR and that may be more complicated with the chunked classloading I am looking at. Still something to think about.

kares · 2019-09-17T07:15:53Z

good, was also thinking of having an insurance policy (jit.max) regardless of other work ...
merging, will put up further work for review (on top of this) - the class-loader sharing piece.

headius reviewed Sep 11, 2019

View reviewed changes

kares added 10 commits September 12, 2019 12:21

[refactor] sync-up block JIT promotion with method impl

12374ed

... using the same "fast" call counter algorithm

[fix] negated condition - disabling JIT when its enabled

7296446

[feat] respect JIT compilation limit (with -Xjit.max)

3cdb6ab

previously compilation was unbounded, JRuby will know use a threshold at which compiling stops - potentially to avoid leaking meta-space

[fix] get JIT excludes to also exclude blocks

ac4898e

that are within/belong to specified exclude targets

unify jit logging a bit - include ex.toString in log

8fafd26

[test] hard-coded jit constants + join assert-less tests

682f110

no need to have volatile (int) callCount

7fbb635

its fine for us to loose some counts from other threads - we always check for >= 0 - setting to a negative value while counting now uses MIN_VALUE and is synchronized

have a isJitEnabled instead of isJitDisabled

dc563b1

kares force-pushed the jit-max branch from ab9a263 to dc563b1 Compare September 12, 2019 10:55

restore logic of only checking threashold == -1

06c5767

(in interpreted method/body) to disable build promotion full builds are also promoted when compilation is OFF

kares added 6 commits September 13, 2019 13:01

[feat] allow JIT parameter setting from JMX

899d49d

[refactor] jit class def into a shared helper

55a786d

so we can re-instantiate byte-code size check in a single place

dry-out jit task - block jits will also increment counter

2ef93d0

since we'd like to know the total number of compiled classes

[refactor] make sure log says what could not compile

d788874

[fix] get block JIT excludes to actually work

2b3e0ae

implementation class will now return enclosing scope module, with updated log messages we already distinguish block/method

[refactor] further simplify jit logging - less args to pass

f7ec88a

kares merged commit 7103c22 into jruby:master Sep 17, 2019

kares added this to the JRuby 9.2.9.0 milestone Sep 17, 2019

kares added the jit label Sep 17, 2019

kares deleted the jit-max branch April 28, 2023 09:17

Uh oh!

Conversation

kares commented Sep 10, 2019

Uh oh!

headius left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

headius Sep 11, 2019

Choose a reason for hiding this comment

Uh oh!

kares Sep 12, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

headius Sep 11, 2019

Choose a reason for hiding this comment

Uh oh!

kares Sep 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

headius commented Sep 11, 2019

Uh oh!

kares commented Sep 12, 2019

Uh oh!

kares commented Sep 14, 2019

Uh oh!

headius commented Sep 16, 2019

Uh oh!

kares commented Sep 16, 2019

Uh oh!

enebo commented Sep 16, 2019

Uh oh!

kares commented Sep 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kares Sep 12, 2019 •

edited

Loading