sagax has quit [Remote host closed the connection]
KeyJoo has quit [Quit: KeyJoo]
byteflam1 has quit [Ping timeout: 245 seconds]
_whitelogger has joined #jruby
byteflam1 has joined #jruby
shellac has joined #jruby
byteflam1 has quit [Ping timeout: 245 seconds]
mat_bug has joined #jruby
rusk has joined #jruby
_whitelogger has joined #jruby
sagax_ is now known as sagax
mat_bug_ has joined #jruby
mat_bug has quit [Ping timeout: 252 seconds]
mat_bug_ has quit [Remote host closed the connection]
mat_bug has joined #jruby
shellac has quit [Quit: Computer has gone to sleep.]
mat_bug has quit []
shellac has joined #jruby
mat_bug has joined #jruby
nirvdrum_ has joined #jruby
mat_bug_ has joined #jruby
mat_bug has quit [Ping timeout: 244 seconds]
byteflam1 has joined #jruby
mat_bug_ has quit []
mat_bug has joined #jruby
lucasb has joined #jruby
mat_bug has quit []
rusk has quit [Remote host closed the connection]
byteflam1 has quit [Ping timeout: 245 seconds]
byteflam1 has joined #jruby
byteflam1 has quit [Ping timeout: 268 seconds]
byteflam1 has joined #jruby
byteflam1 has quit [Ping timeout: 245 seconds]
byteflam1 has joined #jruby
Ryctolagus has joined #jruby
xardion has quit [Remote host closed the connection]
<Ryctolagus>
Wondering what the status of support for OpenJDK is? Can it we hot swapped for Oracle's JDK/JRE? If so which JVM is recommended (if any) HotSpot or OpenJ9?
<rtyler>
win5
<rtyler>
derp
xardion has joined #jruby
shellac has quit [Ping timeout: 258 seconds]
<kares[m]>
Ryctolagus: supported, yes, both
<Ryctolagus>
kares: Thank you, that will please the bean counters here.
<kares[m]>
HotSpot, these days, still performs better with JRuby compared to OpenJ9
<Ryctolagus>
HotSpot it is then :) any reason not to move up to 11LTS, employer is still running 8
byteflam1 has quit [Quit: Lost terminal]
<kares[m]>
8 is fine and pbly still the best option - you will get a different GC default with 11
<kares[m]>
+ JRuby might cause some warnings on Java 9+ ... aside from that its all fine to use
<kares[m]>
* 8 is fine and pbly still the best option - you will get a different GC default with 11
<kares[m]>
\- JRuby might cause some warnings on Java 9+ ... aside from that its all fine to use
<kares[m]>
* 8 is fine and pbly still the best option - you will get a different GC default with 11
<kares[m]>
JRuby might cause some warnings on Java 9+ ... aside from that its all fine to use
<Ryctolagus>
kares: thank you for the information.
Ryctolagus has quit [Read error: Connection reset by peer]
<enebo[m]>
kares: I landed those Rakefile changes. I went through no-task Rake run and it still compiles and anything which uses build as a trans dep as a task will now build with the gem file included
<enebo[m]>
kares: but could you make sure that is ok?
<enebo[m]>
kares: I did a rails 6 rc2 app with sqlite3 and things work too
<kares[m]>
oh yeah? that is nice
<kares[m]>
if only there was a public Rails app we could try for MySQL/Postgres
<kares[m]>
that is tracking Rails upstream
<enebo[m]>
yeah it is a sticking point for the project release where we depend on the testing to tell us it is cool
<enebo[m]>
in fact I also would love it if we could make some standardized simple app for benchmarking as well
<enebo[m]>
probably some docker images or something with the same app
<enebo[m]>
I have not given up on discourse as a project but that does not really fit what we need since it is complicated and pretty tied to a specific Rails version
<rdubya[m]>
Yeah, I'm close to a version that supports mssql with rails 5, but I've unfortunately not had time to work on it this summer
<headius[m]>
the problem goes like this... when we turn a proc into an interface impl, we singletonize it so there's a new RubyClass to which we can attach the impl
<headius[m]>
The only reason for this is because we can't attach that impl to the Proc class of course
<headius[m]>
but by using a singleton class, the singleton now references the proc via "attached"
<headius[m]>
So we have classloader => impl class => method cache => singleton class => proc => binding
<headius[m]>
which causes this "leak
<headius[m]>
what we really want is a new normal subclass of proc that we quickly swap into the object, similar to singletonclass but without the attached reference
<headius[m]>
kares: you may have thoughts here too
<headius[m]>
creating a new class is easy but it wouldnt' be anonymous...the proc would from then on appear to be that new class
<headius[m]>
so first thing I'm trying is setting the attached to something mundane, like the Proc class itself
<headius[m]>
the real non-singleton subclass seems like it might be the better choice though
<headius[m]>
either way if we get rid of that attached link we'll solve all issues of proc-as-interface rooting a bunch of runtime state
<headius[m]>
this also reminded me that the interface impl logic really needs to start using indy all the time
<headius[m]>
it's still using the equivalent of CachingCallSite (via RuntimeCache class)...if it used indy, Java calls through the interface could actually inline the ruby code
<headius[m]>
why are there no intellij plugins that can generate a gist
<headius[m]>
there's a bunch for fetching gists and doing things with them
<headius[m]>
kares: you might enjoy doing the indy bit
<headius[m]>
I'll try a patch that makes a new concrete class
<enebo[m]>
Maybe we should add a method so detach or use some other name than reassign with another attach for this?
<enebo[m]>
oh or make a full class :)
<headius[m]>
yeah
<headius[m]>
I mean I think it's only using singleton class because it's a one-call way to stick a fake class below the proc class
<headius[m]>
there aren't a lot of places in Ruby where we "become" a different concrete class, but this seems like it's appropriate
<enebo[m]>
and the instance of that fake class is held naturally by the proc/interface it is representing
<enebo[m]>
and once it is done it can then collect because it does not have a funky second attach to singleton itself
<enebo[m]>
second reference
<headius[m]>
right
<headius[m]>
it's really all about the attached
<headius[m]>
but we have no other code where insert fake classes other than via singletons
<enebo[m]>
To me this all sounds pretty reasonable. Trying to brainstorm on problems would almost entirely be about losing the primary reference before it is supposed to be gone but I don't know what that could be here
<headius[m]>
enebo: can you think of any problem with the singleton attached being something different?
<headius[m]>
it seems to be used primarily for calling hooks when methods are defined
<headius[m]>
called against the object so the object can react
<headius[m]>
I remember us talking about whether this should be a weak reference too
<headius[m]>
so if the object goes away the reference from the singleton class goes away, even if it's held in a cache somewhere
<enebo[m]>
yeah that would be another angle but it is for a single command-pattern for implementing a proc
<headius[m]>
MRI may not even consider this a hard reference for GC purposes
<enebo[m]>
so I don't really think attached does anything here past be a second reference
<headius[m]>
another angle to think about is the massive overhead for creating these proc impls
<headius[m]>
we are not regenerating the Java impl but we're at minimum creating a new class every time you call
<headius[m]>
with a new proc
<enebo[m]>
well that is true but secondary in nature to being a leak
<headius[m]>
the smart logic would cache the synthetic impl classes we make and reuse them...so if you call with a dozen different procs to something that wants Runnable, they'll all be the same synthetic class
<headius[m]>
yeah
<headius[m]>
just working on this stuff makes me realize how massive the improvements could be
<headius[m]>
but that's 9.2.9 or later
<enebo[m]>
I do agree the followup to this would be to make some site cache
<headius[m]>
optimally it could just be proc.setMetaClass(procPlusInterfacesClassWeFound)
<headius[m]>
and then proceed
<enebo[m]>
or possibly a site cache somehow
<headius[m]>
ah true
<headius[m]>
right now even the indy ruby-to-java calls don't cache any conversion results
<enebo[m]>
then it would be rooted to the code it exists in which could get collected or removed but if not we would just have it
<headius[m]>
they could
<headius[m]>
last time we called this we needed to convert a proc to a Runnable...let
<headius[m]>
let's cache that thing
<headius[m]>
hell, in that case the only cost would be ProcRunnableThing.new(procObject)
nirvdrum_ has quit [Ping timeout: 246 seconds]
<headius[m]>
ProcRunnableThing would be all indy dispatches to proc.call
<enebo[m]>
so repeated uses means hitting the same code over and over so we could consider IRScope for storing live data (or maybe StaticScope depending on how you think about this stuff)
<headius[m]>
other angle would be making the IRScope for that proc body cache it
<headius[m]>
then we could even have the impl class dispatch directly to the body
<headius[m]>
rather than via proc.call
<headius[m]>
sigh
<enebo[m]>
It would leak once but if the code is still reachable then that is probably fine. If we really cared we could stuff it behind soft/weak reference
<headius[m]>
yeah we both thought of it
<enebo[m]>
funny if we both read each others shit we would save a lot of typing :)
<headius[m]>
if only we didn't have all these pesky user issues to deal with
<enebo[m]>
In any case for 9.2.8.0 I think making this type is ok and the idea of live scope saved data goes well beyond even this particular case
<enebo[m]>
It could be for anything in the scope we want to not recreate
<enebo[m]>
Although if we made it too generic it may be icky to debug when we stuff the wrong thing away and then wonder why we are storing fooberries
subbu|lunch is now known as subbu
<headius[m]>
hmmm
<headius[m]>
it's hard to get this to be a concrete class
<headius[m]>
there's logic above the getSingletonClass that extends the Java interfaces into the hierarchy
<headius[m]>
that's really where the singleton is created
<headius[m]>
and a concrete class can't extend a singleton class, so I have to move that around a bit
<headius[m]>
it's more invasive anyway
<headius[m]>
I almost have a patch that I think is ok
<headius[m]>
if you already have a singletonized proc I just use the singleton class
<headius[m]>
if the metaclass is not a singleton I stick a new anon RubyClass under it and make that the metaclass
<headius[m]>
the Java interfaces will not go through extend anymore, instead just metaClass.includeModule
<headius[m]>
so that's a somewhat visible change
<headius[m]>
ah fudge
<headius[m]>
without extend logic it doesn't create those dortch methods
<headius[m]>
__jcreate_meta! and stuff
<headius[m]>
this stuff is also still grossly entwined with JavaObject logic
<enebo[m]>
why do those methods still exist?
<enebo[m]>
or maybe I mean why does __jcreate_meta!
<headius[m]>
that one I'm not sure, I don't remember what it does
<headius[m]>
others I know are to handle dual-initialization via initialize and Java constructor
<headius[m]>
but that shouldn't apply here...the object is already
<headius[m]>
wait
<headius[m]>
oh
<headius[m]>
hmm ok I dunno
<headius[m]>
this is the only place we call it
<headius[m]>
o_O
<enebo[m]>
// Used by our duck-typification of Proc into interface types, to allow
<enebo[m]>
// coercing a simple proc into an interface parameter.
<enebo[m]>
clazz.addMethod("__jcreate_meta!", new InterfaceProxyFactory(clazz, "__jcreate_meta!"));
<enebo[m]>
yeah it is only for this feature
<headius[m]>
yeah so basically it mixes in the Java interfaces via extend, which in turn triggers the generation of the actual Java class and the creation of the proxy object
<headius[m]>
or something like that
<enebo[m]>
haha we could just do newInfterfaceProc(self) and remove the dyncall
<headius[m]>
at the bottom of convertProcToInterface it calls __jcreate_meta! to get the real Java object to pass
<enebo[m]>
I see nothing which overrides this logic either
<headius[m]>
yeah I can try that
<headius[m]>
this was over-dortched I think
<enebo[m]>
well to be fair we had a lot of Ruby impl in JI back then
<enebo[m]>
this is likely just some code we moved 1/2 from ruby->java then moved the other 1/2 later and never realized there is not actual dynamic or polymorphic behavior here
<enebo[m]>
kares: ^ You may know this answer
<headius[m]>
could be
<enebo[m]>
I did a search on ruby and there is nothing
<enebo[m]>
in Java it is seemingly for only that call
<headius[m]>
ugh it returns JavaObject
<headius[m]>
that whole pyramid needs to die in a fire
<headius[m]>
so there's many layers of waste here
<headius[m]>
ok next problem
<headius[m]>
extend logic puts the interfaces in hidden interface list that newInterfaceProxy uses to generate the impl
<headius[m]>
it calls getInterfacesFromRubyClass in Java
<headius[m]>
@java_interfaces
<headius[m]>
I think appendFeaturesToClass in JavaInterfaceTemplate might be the thing to call to add the interfaces
<headius[m]>
but this rabbit hole is getting pretty deep
<enebo[m]>
yep
<headius[m]>
so the logic now will basically be
<headius[m]>
1. create new synthetic class below proc's class
<headius[m]>
2. append Java interfaces into it as if it were any other concrete class
<headius[m]>
3. use newInterfaceProxy directly to generate the proxy object
<enebo[m]>
I don't see a downside of killing __jcreate_meta! at this point as a separate item...not sure on _jcreate itself but the meta can be a call to a single static method at that point
<headius[m]>
I do not either so far
<headius[m]>
well I successfully ran my little script
<kares[m]>
no idea on `__jcreate_meta!` really, know another __jcreate but the iface one rings no bell
<headius[m]>
so my changes are preventing the other way of creating an interface impl from working
<headius[m]>
Interface.new { }
<kares[m]>
looks reasonable on first look - can actually understand what you meant with the comments above
<kares[m]>
yeah you could run into troubles ... there's also Iface.impl but that one hopefully does not use much of this code
<enebo[m]>
two different errors
<headius[m]>
I'm leaning toward shipping the "attached" hack or nothing in 9.2.8
<headius[m]>
definitely not this
<enebo[m]>
the one which cannot find 'x' is the scariest one
<headius[m]>
I think we can unravel this for 9.2.9 but this sweater comes undone fast
<headius[m]>
kares: you see attached hack above?
<kares[m]>
+1
<enebo[m]>
I believe we can solve jcreate_meta! today
<kares[m]>
yep
<headius[m]>
I can point that at anything...even a bogus new object
<enebo[m]>
I cannot conceive of how removing that as a dynamic dispatch could break anything other than a mad scientist somwhere but I highly doubt there is one since none of us knew what it was even for
<headius[m]>
ahh
<headius[m]>
initInterfaceImplMethods
<headius[m]>
uses the presence of __jcreate_meta! to know if it should do some stuff
<headius[m]>
if (!(clazz.isMethodBound("__jcreate!", false) || clazz.isMethodBound("__jcreate_meta!", false))) {
<enebo[m]>
yeah seemingly just java_class though
<headius[m]>
🤷🏻♂️
<headius[m]>
right so this is a mixed up bit of logic
<enebo[m]>
and only in some cases :)
<headius[m]>
for Ruby classes that implement an interface we also have to wire up the constructor/initialize dance
<enebo[m]>
unless this always triggers somehow
<headius[m]>
we don't have to do that for procs that already exist
<headius[m]>
why does this look for jcreate_meta
<enebo[m]>
So __jcreate! OR __jcreate_meta! trigger defining themselves and also adding java_class
<headius[m]>
yeah
<headius[m]>
no
<headius[m]>
if ! jcreate || yes jcreate_meta
<enebo[m]>
My next question would be can __jcreate! already exist and __jcreate_meta! does not
<headius[m]>
so if jcreate is not there OR if jcreate meta IS there
<headius[m]>
it creates jcreate
<enebo[m]>
HAHAHA wow this is complicated
<headius[m]>
jcreate meta as we've seen seems to only be for lazily generating the impl class for procs
<enebo[m]>
I just noticed the java_interfaces hook
<headius[m]>
but this also uses InterfaceProxyFactory method for jcreate
<headius[m]>
so I'm thinking for procs I can just skip this whole block and remove the jcreate_meta check
<headius[m]>
since I'm making the interface proxy directly now
<enebo[m]>
I mean we can eliminate the dyncall but I don't know about removing any of this logic in initInterfaceImplMethods without some serious code reading
<headius[m]>
yeah exactly
<headius[m]>
and this isn't all of the code either
<enebo[m]>
If __jcreate! is never defined without __jcreate_meta! we could actually delete it but I don't know that
<enebo[m]>
It would appear the opposite is true though !jcreate || jcreate_meta! but this case is not relevant
<enebo[m]>
My real question is what is this actually testing?
<headius[m]>
I think we need to take a conservative path to removing this because I don't know the answer to that question
<enebo[m]>
yeah
<headius[m]>
right
<enebo[m]>
but that looks far from trivial
<headius[m]>
it is dispatched but only once right after setting up the proc interface
<enebo[m]>
so yeah let's for sure remove the dyncall but leave all this shit here maybe with a FIXME comment?
<headius[m]>
so the actual method go away in favor of a flag
<headius[m]>
that would be a first step
<headius[m]>
ok yeah, I can remove the dyncall...but then should we ship the attached hack?
<headius[m]>
or ship nothing
<enebo[m]>
good question...try the hack and see how our tests run
<headius[m]>
the hack is green
<headius[m]>
JI and jruby tests
<headius[m]>
the RubyClass subclass patch works for the original case but breaks a bunch of other interface impl paths
<enebo[m]>
yeah I am wondering if anything else in our suite uses JI enough to do something we may not test directly
<headius[m]>
I will push as a PR and we can see the rest of the suites
<enebo[m]>
but with that said I am not sure if we can be sure that our tests are enough to know
<headius[m]>
embedding might hit more of it
<headius[m]>
and we do use JI stuff to impl some ruby features
<enebo[m]>
sequel and maybe mkristians pangea of webserver tests
<enebo[m]>
or do we no longer run that
<enebo[m]>
I actually don't even know what that job does beyond download dozens of artifacts
<enebo[m]>
Not having attached to proc means what? That is our qualitative decision on this. What could break?
<enebo[m]>
I like the idea of landing this because it is a fairly obvious leak and it would be nice to correct it
<enebo[m]>
doing it last minute maybe is dumb though
<enebo[m]>
unless we can reason through this and obviously see no failures in tests
<enebo[m]>
OTOH we have had the leak for a long time now...punting to 9.2.9 will not kill anyone
<enebo[m]>
For me punting only comes from having a reasonable amount of doubt
<enebo[m]>
reasonable == I am feeling frightened :)
<headius[m]>
we could ship nothingin 9.2.8, this has just dragged on a long time and it's ugly
<headius[m]>
the attached hack is pretty light as far as hacks go
<kares[m]>
believe __jcreate is used wout __jcreate_meta ... (unless I am wrong) to boot up Java sub-classes in Ruby
<headius[m]>
looking pretty green so far
<headius[m]>
I have one change to this: if it's a singleton before we get here DON'T detach
<headius[m]>
someone out there has a singleton proc they're passing to Java
<headius[m]>
maybe only do our hack if proc.getMetaClass == runtime.getProc()?
<headius[m]>
so if it's a natural proc, singletonize + clear attached
<headius[m]>
that covers most cases where this would be a problem...basically all cases where you are taking a literal block and turning it into an interface impl
<headius[m]>
kind of amazing that base JRuby startup is only about 10MB of heap now...we've improved a lot
<kares[m]>
some details on changing atached might not work right, such as method callbacks?
<headius[m]>
yeah that seems to be the only thing?
<kares[m]>
`proc.singleton_method_added` will not work for such proc
<headius[m]>
but if I only do this for natural procs, there's no singleton methods to hook back to
<headius[m]>
and if someone makes a singleton proc on their own I won't do this
<headius[m]>
(pushing that additional change now)
<kares[m]>
yeah just realized that was your reason on the added check
<kares[m]>
a code comment would be nice next to the check ;)
<headius[m]>
that doesn't address any math perf degradation in interp, but it fixes it in JIT and always uses indy then
<headius[m]>
maven connection lost
<enebo[m]>
headius: I don't know what but we definitely need something to measure relative change
<enebo[m]>
headius: our simple rails app numbers were not that bad so I wish I better understood how we fall over on this
<enebo[m]>
Someone said it took like 18 hours to warm up
<headius[m]>
yeah that's pretty nuts
<enebo[m]>
which I am guessing is a massive rails app with lots of controllers etc...
<headius[m]>
that was with indy?
<headius[m]>
or just in general
<enebo[m]>
regardless though I think just making changes and running a small app to measure perf cannot give us this dimension of data
<enebo[m]>
I thought in general because the context was we were using compile.invokedynamic in our slides
<headius[m]>
need input
<enebo[m]>
yeah
<headius[m]>
you can run rg.org I guess and see if you see anything
<headius[m]>
that patch is wrong btw, I need to pull up the indy logic from IRBC7
<enebo[m]>
well so kares and I were discussing earlier that we really need some common rails app which is not too version sensitive so we can test all the dbs for arjdbc
<headius[m]>
redmine would be a way nicer example app if we can get it working
<headius[m]>
it used to work fine
<headius[m]>
it's self-contained and doesn't integrate a lot of weird external stuff like discourse and RG do
<enebo[m]>
I feel like we should put some stake in the sand on what is important to test in rails perfwise and then not only measure tps but also memory and warmup
<headius[m]>
yeah and start auditing that over time
<enebo[m]>
but redmine may not work across multiple rails versions which I know is a weird requirement
<enebo[m]>
I guess I should not conflate the two projects requirements though
<enebo[m]>
Having one common rails version over time is good for comparison
<headius[m]>
I don't see that as a problem
<headius[m]>
if we need to update our redmine bench at some point, yeah, it would cut historical timeline in half...but we can say "ok, this is the new baseline"
<headius[m]>
we're mostly looking for improvement going forward and no regression
<enebo[m]>
It definitely would be great to use something real
<headius[m]>
and ideally "current" jruby releases should be able to run an older redmine for a long time
<enebo[m]>
RG.org partially has the issue that we need to populate gem data
<headius[m]>
yeah maybe there's a demo redmine DB we could use
<enebo[m]>
redmine needs issues but that could be one big fixture where we can vary data
<headius[m]>
I know folks who sold packaged JRuby+Redmine as an app
<headius[m]>
I'm sure we could find some data load to use
<enebo[m]>
well nice data makes it much more appealing if it exists
<headius[m]>
I'm on board with doing the patch above for 9.2.8 at this point
<headius[m]>
I can toss that into a PR if we want to see full CI but we have been testing a few suites with indy for a while
<headius[m]>
how many math sites did you say you saw?
<enebo[m]>
redmine is a 5.2.3 app atm so not bad
<headius[m]>
yeah
<enebo[m]>
in my gem list nearly 4000
<headius[m]>
4000 different sites?
<enebo[m]>
yes
<enebo[m]>
remember though I have quite a few gems and some of those likely are from gemspecs
<enebo[m]>
I think I still have the code for that stashed...now that I have rails 6 rc2 simple app I will see how many in doing aconsole hit
<enebo[m]>
but == and >= are super common
<headius[m]>
ah true
<headius[m]>
I forget about boolean and bit operators
<headius[m]>
hmmm
<headius[m]>
additional snag with my patch
<headius[m]>
without indy enabled we don't use switchpoint to invalidate class hierarchy
<headius[m]>
so it breaks
<headius[m]>
so we'd need to turn on switchpoint globally to make the change above
<enebo[m]>
interestingly something on Java 11 won't bootstrap rails from loading bindex
<headius[m]>
which is much more likely to have a warmup/startup impact
<enebo[m]>
recompiliung jruby on 8 and running on 8 to look at callsite counts
<headius[m]>
ok
<headius[m]>
I'm back to thinking we should revert kares original patch and prepare a new one for 9.2.9
<headius[m]>
we can debate whether that optimizes interp or not
<enebo[m]>
echo exit | rails c has 3597 callsites
<enebo[m]>
single model/controller rails 6 app
<headius[m]>
I suspect most of those get called since we wouldn't compile to IR (and create the CallSite object) unless the surrounding method is called
<enebo[m]>
yeah I have not traced through how these get constructed but we do definitely make them in CallBase on construction
<headius[m]>
non-indy jit will create them on first call
<headius[m]>
IR creates them eagerly during compilation
<headius[m]>
indy doesn't use them of course
<enebo[m]>
headius: you mean the fast callsites or a callsite at all?
<headius[m]>
indy does not use anything that descends from our CallSite
Ryctolagus has quit [Quit: Leaving]
<headius[m]>
for this anyway...I think there's some unoptimized calls that do
<enebo[m]>
headius: I am only reporting the construction of those sites
<enebo[m]>
not actual visits
<enebo[m]>
so many of these may or may not get called
<headius[m]>
right
<headius[m]>
I'm just saying the ones created by jitted code will have been called at least once
<headius[m]>
but you can't see that in this number
<enebo[m]>
ah yeah true
<headius[m]>
CallBase could probably lighten up and create site lazily too
<headius[m]>
there's no race involved
<enebo[m]>
at the point of rails c number jitted is not really an important stat other than affecting startup time
<headius[m]>
paths that never get called would never create a site
<headius[m]>
I just had a bad idea
<headius[m]>
make a caching call site mixin using an interface with all default methods
<enebo[m]>
yeah that is a good point and that is an easy win
<headius[m]>
so any object can mix in a monomorphic cache
<headius[m]>
I've wanted that in many places
<headius[m]>
I've also wanted to make RubyEnumerable into a mixin interface but that's a bit more severe
<enebo[m]>
heh
<enebo[m]>
This makes me want to figure out how many scopes are actually executed
<enebo[m]>
initializing callSite on demand means adding a callsite == null check for every interp call
<enebo[m]>
which is not a horrible cost but still
<headius[m]>
branch prediction probably moots that
<headius[m]>
null check is pretty fast
<enebo[m]>
just preprocessing a interpreter on first call to a scope to just resolve all callsites first time scope is executed would still provide a lion share of memory savings but then have no null check
<enebo[m]>
well 1 per scope execution
<enebo[m]>
1 comparison of some kind
<enebo[m]>
yeah maybe so
<enebo[m]>
I generally don't complain about null checks but I guess it is very unlikely to be visible with the other noise of the IR interp
<enebo[m]>
last time we unboxed args we could not see that impact
<headius[m]>
gah...brakeman needs ruby 2.3+ now
<headius[m]>
I'm not sure how to test this perf bug
<headius[m]>
ah I'll force the version in the bug
<headius[m]>
nope
<headius[m]>
depends on rb_inotify which now requires ruby 2.2
<enebo[m]>
lol I am reading through that issue again
<headius[m]>
I think we might be faster now!
<enebo[m]>
yeah that's right most of this is purely from poor performance with a huge method
<headius[m]>
a method we don't jit
<enebo[m]>
have 8 point releases of changes since last run
<headius[m]>
damn
<headius[m]>
brakeman 3.2.1 errors on 9.2.8
<headius[m]>
and brakeman 4.6.1 doesn't install on 1.7.27
<headius[m]>
well, I'm mostly concerned with the per regression
<headius[m]>
I don't know if this comparison is valid, but it's looking better
<headius[m]>
I'll try MRI
<enebo[m]>
You have virtually identical time
<enebo[m]>
I predict 13 c
<enebo[m]>
OH YES
<headius[m]>
MRI:
<headius[m]>
real0m15.865s
<headius[m]>
user0m15.081s
<headius[m]>
sys0m0.518s
<enebo[m]>
time jruby -Xcompile.invokedynamic -S brakeman -q --summary redmine
<enebo[m]>
NoMethodError: undefined method `exception' for #<Java::JavaLang::ClassCastException:0x23bd2f6e>
<enebo[m]>
I got 13s but my redmine is not on HEAD
<enebo[m]>
indy bug
<headius[m]>
where 9.2.8 with same brakeman took 28s
<headius[m]>
but a 28s run is pretty cold
<enebo[m]>
yeah I got 28 and 13
<headius[m]>
I'm sure things have changed in brakeman but would that explain 50s => 28s improvement vs 1.7?
<enebo[m]>
but no doubt redmine sized app is maybe not an unreasonable command line for perfing
<headius[m]>
that's a pretty big leap[
<enebo[m]>
how does MRI run the old version?
<headius[m]>
oh good idea
<enebo[m]>
if it also halves then you he did something neat :)
<headius[m]>
it fails the same way we do
<headius[m]>
so I can't test it
<headius[m]>
(and I know now that's not our bug)
<headius[m]>
I'll add these numbers but maybe we can close this?
<enebo[m]>
it is too stale
<enebo[m]>
I think if we wanted to open an issue it would be on huge methods
<enebo[m]>
but in that case I don't feel we need an issue
<enebo[m]>
not unless friday funday solves it
<enebo[m]>
I do believe there is a solution to this with virtual ICs and a shared temp var table but my mind can conceive of it but I am not sure how far down the pit it will go
<headius[m]>
too stale = close it?
<headius[m]>
I mean...the only comparisons we can really make now seem to show we're doing fine
<enebo[m]>
I think we can close and write a comment saying lets open a new issue based on current perf issues
<headius[m]>
even if brakeman has improved since then, 9.2.8 is now < 2x slower
<headius[m]>
not even 1.7 got that close
<enebo[m]>
and see what he said
<enebo[m]>
well a little over 2x slower on my machine
<enebo[m]>
less on yours
<enebo[m]>
but it may be noisy
<enebo[m]>
or if I pull-rebase perhaps I still see it somehow improve
<enebo[m]>
what he says
<headius[m]>
sure...last posted comparison was comfortable above 2x for both
<headius[m]>
comfortably
<enebo[m]>
ok so we probably gained a bit
<enebo[m]>
makes sense
<headius[m]>
this bug was specifically about 1.7, so I call it done
<headius[m]>
I'll close it and ask them to open a new issue if they want to continue pursuing the MRI numbers
<enebo[m]>
okie dokie
<enebo[m]>
yeah and they obviously made some amazing change
<enebo[m]>
or I am guessing they did
<headius[m]>
yeah hard to say
<headius[m]>
May 15 2018...I would have had the same machine I have now and it was 50s 9k, 40s 1.7
<headius[m]>
I don't have MRI numbers from this machine on Brakeman 3.2.1
nirvdrum_ has quit [Ping timeout: 245 seconds]
lucasb has quit [Quit: Connection closed for inactivity]