Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[jit] replace method invocation counter with jit.time.delta #5887

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kares
Copy link
Member

@kares kares commented Sep 24, 2019

this is @enebo's work from https://github.com/jruby/jruby/tree/counter_nuke
rebased + with a few touches for usability (able to specify jit.time.delta as long)

might need more work as we still do not have real-world numbers (hopefully soon).
... or we can add it into next 9.2 but having jit.time.delta = -1 (disabled) by default?

enebo and others added 3 commits October 30, 2019 18:05
…been

reached BUT it has taken longer than -Xjit.time.delta ns since the last
time
saved when the counter was set to 0.

This is totally untuned in that I made up a number and it I suspect is
too
dramatically not JITting methods.  In running an oj bench and gem list
(with
and without --dev) and starting up rails the times do not appear to be
much
different.

After tuning the value (which I guess would be for this machine -- a
follow
up to this work will be whether one magic constant can work here) which
would
be figuring out proper time delta and also determing whether any
important
methods which should have JITted had not there is a followup change I
want to
try: change threshold to be more than just a method call counter.  I
would
like to also add threadpolls as an increment value.  Then larger methods
which
are not called as much will end up as JIT candidates.
and also a way to disable System.nanoTime checking when < 0
@kares
Copy link
Member Author

kares commented Oct 30, 2019

should be ready now, it could use an com.headius:options release to get rid of the LongOption class

@enebo enebo modified the milestones: JRuby 9.2.10.0, JRuby 9.3.0.0 Feb 11, 2020
@enebo
Copy link
Member

enebo commented Feb 11, 2020

I believe this is safe but without pushback I am skipping for somewhat imminent 9.2.10.0.

@headius
Copy link
Member

headius commented Jul 1, 2021

I merged latest master into this to see where we stand on it, but I don't see us making this change right before 9.3.

@kares
Copy link
Member Author

kares commented Jul 5, 2021

we did not do a lot of experiments with jit.time.delta, but the feature could be off by default (as presented in the PR).
the added overhead (when off) is simply the nanoTime() call on every method instantiation ...

@kares kares changed the title [WiP] counter nuke with jit.time.delta [jit] replace method invocation counter with jit.time.delta Jul 5, 2021
@headius headius modified the milestones: JRuby 9.3.0.0, JRuby 9.3.1.0 Aug 19, 2021
@headius
Copy link
Member

headius commented Oct 11, 2021

Untargeting until we have a specific release in mind. @kares Let us know what you think we should do here.

@headius headius removed this from the JRuby 9.3.1.0 milestone Oct 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants