<ahorek[m]>
but I think it's a false positive. Since the counter should be always acessed by the current thread owner. headius ?
<headius>
yo
<headius>
re: OOM, yeah it seems to have gone away with more metaspace. We used to bump that up at the drop of a hat but have been more cautious in recent years. We can continue to monitor but it may be a non-issue
<headius>
specs continue to grow and our code generation evolves over time so we could easily just be generating more metaspace data now
<headius>
re: coverity, I agree it probably should be fine but adding an atomic accessor getAndIncrement would boil down to fast code for same thread and avoid the issue
<headius>
at a glance it looks weird but you are right that it should never get incremented except by the owning thread
<headius>
you know, if the jnr release process didn't take so long I wouldn't get distracted as easily
<mattpatt[m]>
I'm trying to do something with a URLClassLoader - I have a JRuby system that's a long-running pipeline processor and now I need to sometimes execute jobs that load something from a JAR, and I need a class loader because it's entirely possible that multiple versions of the same JAR will be in play. The Jar I want to load has dependencies. Is there a relatively sane way to combine this Class loader and maven at runtime, or should I just
<mattpatt[m]>
commence weeping now?
<mattpatt[m]>
Maybe I just need to go to squeak and demand Fat JAR be built for anything that could conceivably be wanted to be used in this way
<headius>
mattpatt: hello!
<mattpatt[m]>
hey
<headius>
yeah that sounds complicated
<headius>
if you managed your own classloaders and yanked code from them directly you might be able to get it to work but I think no matter what there's going to be a lot of manual work
<headius>
I'm not sure exactly where the classloaders fit into JRuby
<headius>
you classloaders I mean
<mattpatt[m]>
The whole system runs is written in Ruby and runs on JRuby
<headius>
by default JRuby will load external jars into a single JRubyClassLoader (extends URLClassLoader) per runtime so the isolation would be additional logic on your end and might cause some peculiar java integration issues
<mattpatt[m]>
yeah
<headius>
I can't say I've heard of anyone running a JRuby rig that might have two versions of some Java class in flight
<headius>
at least not within a single JRuby instance
<mattpatt[m]>
I imagine I need to sandbox all the Jar's dependencies inside the same ClassLoader and make sure classloader delegation doesn't bite me
<headius>
yes at least that
<headius>
you might have to isolate the JRuby instances based on those jar clusters as well since we may not know a v1 FooBar from a v2 FooBar everywhere in the Java integration code
<headius>
it should be based on the actual Class object, which would differ by classloader, but it has never been tried that I know of
<mattpatt[m]>
In theory the ClassLoader for each cluster can be short lived - only needs to exists for as long as the job is running - and can be destroyed at the end of the job so hopefully that helps
<mattpatt[m]>
fortunately all the expected uses for this are utility converter classes that need testing
<mattpatt[m]>
so pretty isolated string in - string out kind of things
<headius>
well it should be possible to do it manually, but it will be interesting to see what you run into 😀
<mattpatt[m]>
woo
<headius>
having a common interface outside of the child classloaders will make it cleaner, since you'll always be calling against a known interface from JRuby
<headius>
if that is possible to do
<headius>
I also make no guarantees about GCing those classes and classloaders since Java integration guts can get kinda sticky... we try to make it possible for classes to GC but that is assuming our usual classloader structure
<mattpatt[m]>
i'm not worried as much about GC, fortunately - the instances themselves only live as long as there are jobs to process
<mattpatt[m]>
i wouldn't be worrying about it at all except that an instance will process hundreds of jobs, and the idea of shelling out to a new JVM for each job makes me sad, and would add huge amounts of time to the processing...
<headius>
if the spin-up time for JRuby itself is not a concern (instances after the first start up much faster) isolating that way is a possible fallback option
<mattpatt[m]>
and the person waiting on the job has to assess correctness of output, so interactions between older/newer converter and the XSLT built for the other version is guaranteed to generate utter undebuggable confusion if it does happen...
<mattpatt[m]>
there'd be no JRuby startup at all - the converters are all pure-java - so they'd be probably much quicker
<headius>
ok
<mattpatt[m]>
i think i will try just shelling out first and see if it's acceptable
<headius>
ok jnr update releases should be out... gonna take care of a late lunch while they propagate and then we can update JRuby proper
<enebo[m]>
heh this optimization had an error where I would delete outgoing CFG edges but not remove the incoming edges from the destination BB...but...it made me find a whole new weird thing
<enebo[m]>
defined? self.some_method will create a b_nil(label, "self")
<headius>
incoming edges kept those blocks from being culled I assume?
<enebo[m]>
which we now go "oh wait that is dead code"
<headius>
oh weird
<enebo[m]>
but I am wondering when is defined? going to return a nil to begin with
<enebo[m]>
I think it may be just defined? to_not_exist perhaps
<headius>
it returns nil when whatever is points at is not defined or otherwise an opaque expression
<headius>
or is it false?
<enebo[m]>
but without fixing my CFG issue I just realized we can statically examine this result and kill the first obvious branch which is mostly not needed
<enebo[m]>
heh well good question
<headius>
yeah self will never be undefined
<headius>
and if it is nil that doesn't matter
<enebo[m]>
but no undef specifically nil
<enebo[m]>
b_nil so I guess defined? logic returns nil on stuff that is not there?
<enebo[m]>
which seems weird also
<headius>
yeah should be nil, because the positive result is a string describing the kind of thing
<enebo[m]>
anyways our defined? code seems to work so this is just an oddity more than anything. I can prune some branches no matter what though
<headius>
it's really more "what kind of thing does this code resolve to"
<enebo[m]>
yeah
<enebo[m]>
which is a string representing what or nil if not
<enebo[m]>
I do find it interesting as an internal it is not just undef but I suppose in the end it wants to be nil
<enebo[m]>
headius: as to your earlier question about incoming edges preventing something...nope the problem is when optimize in CFG runs it asks for incoming edges and we removed it from the other direction so it gets a NPE
<headius>
hah ok
<enebo[m]>
weirdly this does not happen for the b_false case in kpeg. I think something larger is happening because of the original elimination which probably is a bb is getting merged
<enebo[m]>
if only defined? was a hot path anywhere :)
<headius>
it might be but I usually only see it in metaprogramming
<headius>
one of those weird older features I'm not sure everyone realizes how weird it is
<enebo[m]>
oh yeah it is conceivable but if it is hot code people probably are not doing it
<enebo[m]>
yay I hope this works...dirgra has a removeEdge(Edge) which is exactly what I want
<enebo[m]>
hmm
<enebo[m]>
nope
<enebo[m]>
doh..I see what is wrong..I stored returnBBs as a list in CFG as part of this change and in this case a BB it points back to is no longer there
<headius>
oops
<headius>
JNR update pushed and seems to be working
<headius>
it must be something pretty subtle... as far as I can tell unrescuable should propagate the same way it did before
<headius>
woo missing rdoc version got released
<enebo[m]>
yay I found it. It was the returnBB list. When orphaning a dead BB I needed to remove it from that list
<enebo[m]>
In the massive yak shave of this opto I notice that we can coalescence two BBs and we don't...
<headius>
so many opportunities
<enebo[m]>
this may be directly from this opto that it is not happening
<enebo[m]>
these bbs used to be unmergable but now are but I can see optimize is not performing it.
<headius>
rdoc update PR repushed, should be good now
<headius>
byteit101: will you have any time this week to help us look into that last issue? That is pretty much the only thing blocking 9.3 now
<headius>
I am of the opinion that we could ship without this since we are sure to have other reports after release
<headius>
reports we will never get until we release
<byteit101[m]>
Yes, excepting tomorrow early afternoon
<headius>
excellent!
<byteit101[m]>
>other reports after release
<byteit101[m]>
I agree, given how much I touched
<headius>
I need to review what's in that issue already, but have we isolated a cause yet?
<enebo[m]>
Ah I am wrong these cannot be collapsed because the runtime_helper instr requires an exception region
<enebo[m]>
that is not super clear once the CFG is made (or it is but I have not looked at a problem in CFG for a while)
<headius>
we should probably split up RuntimeHelper into more fine-grained operations
<headius>
it is a weird opaque "I'M DOING SOMETHING DANGEROUS" instruction
<headius>
n
<byteit101[m]>
given r1 <r2 <j1, iirc, super in r2 has self = r1, so it goes to super of r1, which obviously loops
<enebo[m]>
headius: It is also an option but we also have to encode it for serialization and we try to make all instrs be emittable as a single byte
<enebo[m]>
we have gotten rid of a couple of instrs I should check on the count
<headius>
enebo: a few catch-all instrs like "PerformFrameOperation" with subtypes could avoid bloating up instrs too much
<headius>
I just don't like that all of those RH operations get pegged with the same side effects, regardless of what they do
<headius>
byteit101: I suppose first step would be to try this on current master... there have been some method lookup changes in the last few weeks
<enebo[m]>
Actually I think merge_kwargs and is_hash_empty are the only two that do not have the same operations
<headius>
not sure if they help this case or not
<enebo[m]>
they could be moved out
<headius>
enebo: kwargs especially will want special treatment once we optimize the hash away
<enebo[m]>
headius: those two are both my fault
<enebo[m]>
I should not have added them because they do not have the same operations
<enebo[m]>
but we could make 4 instrs from the complete set: is_defined, handle_flow_control_change, merge_kwargs, is_hash_empty
<enebo[m]>
I will have to do a tally on full count I guess but I am pretty sure we dropped a couple of instrs in the last 2 years
<headius>
yeah
<enebo[m]>
The other thing I have disliked about this catch all is that it does lots of operand length stuff
<enebo[m]>
since not all instrs use same number of operands
<enebo[m]>
so I can probably do this after we get out 9.3
<enebo[m]>
The other instr work I wanted to do was merge inheritance of lexical constant search into a single instr
<enebo[m]>
we gain zero analysis benefit for them being apart
<headius>
yeah it really does nothing for us
<enebo[m]>
that one is a tad more important than runtime helpers because it will reduce instr size quite a bit for Rails (as an example) since there are long constant chains
<enebo[m]>
So the idea to collapse the two is to collapse entire chains and make the isntr variadic
<enebo[m]>
A::B::C::D::E will just be a single instr with no temps
<headius>
how much are those used in practice?
<headius>
class Foo; Object; end, for example, just uses SearchConst that does both
<headius>
same within `def foo; Object; end`
<headius>
I don't know when those are actually used
<enebo[m]>
yeah you there are a couple of search combos
<headius>
for a single-level bare const lookup we use search_const
<headius>
for A::B we use inheritance search
<headius>
when do we use lexical alone?
<enebo[m]>
yeah I am mixing up concerns here
<enebo[m]>
we use lexical in only one place which is defined
<headius>
stacked const lookups definitely would be better to condense, no doubt about that
<headius>
ahhh I see
<headius>
defined does the two pieces separately
<enebo[m]>
yeah I meant that...I just keep remembering about this other instr and mix it into the idea
<headius>
ok
<headius>
something odd going on with the JNR update PR
<enebo[m]>
this lexical only happens for first constant in a defined chain and then inheritance after that
<enebo[m]>
This is pretty maddening when I think about how weird defined is
<headius>
the openj9 Java 8 "mvn test" and the JDK8 "mvn test" are hanging
<enebo[m]>
I was going to say I can merge this behavior into the idea but I guess I should just make the defined instr variadic and merge those two into one as well
<headius>
almost nothing changed in this PR except removing the junit dep from jnr-ffi