<enebo>
I fixed an issue where 'o' Marshal.load objects should not call getConstant as it will search inheritance hiearchy (AS unit test failure where it finds wrong constant after a delete) and it will call const_missing
<enebo>
MRI basically does getConstantAt
<enebo>
So everything is happy Ruby-wise except Java integration
<enebo>
you know wait a second...I may be wrong here...give me a few...this is probably not a problem with magic constants
<kares>
ah so that is what spec:ji is about
<enebo>
although I think we still may have an issue with magic constants if we load a marshalled string before accessing the Java class we are loading
<enebo>
kares: what I was going to mull over with you was the idea we should make JI objects use something else or not
<enebo>
This seemed like it would be so simple to fix and I even limited the change so only marshal load had the new behavior
<enebo>
I would argue we should constantAt for all uses with uncertainty that perhaps const_missing is useful sometimes
<enebo>
kares: so definitely the change for marshal to call getConstantAt breaks things and I suspect it is because we no longer will fire const_missing
<enebo>
kares: fwiw the specific bug I was fixing was not from firing const_missing but I noticed that const_missing never fires in MRI
<enebo>
kares: the problem I fixed was A::C and A::B::C where C from A::B is deleted....then an unmarshal of "A::B::C" gets "A::C" because getConstant searches down and sees a C vs generating an error
<enebo>
I can form-fit here for now and maybe add an explicit const_missing and fix the problem I was trying to fix while still maintaining us doing const_missing for Marshal.load
<enebo>
In the grand scheme I think I would prefer we do something more specific with JI objects and marshalling
<enebo>
going to quick try getConstant with inherit = false to see if all things pass
KeyJoo has joined #jruby
<enebo>
kares: ok I think if I switch to useing c.getConstant(str, false, true) AND reinstate those NameError checks I can fix the issue I was trying to fix and make all tests pass but it will keep in place that our marshal.load will call const_missing while in MRI it wont...so I am hoping we can brainstorm a fix for JI so we can remove that incompatibility
<enebo>
hmm well I may have to retag on spec I fixed as well...part of all this is that we do fail to load() but we just raise the error differently
<enebo>
s/on/one/
<kares>
not sure how problematic it was, but the spec:ji only seem to have failed with the inner cases such as enum constants
<kares>
esp. enums that have overrides -> get a Java class per constant
<kares>
those, I believe, are now hidden in later Java so maybe they do not matter much
xardion has quit [Remote host closed the connection]
<kares>
disclaimer: have not looked into the failures much just a quick look earlier today
xardion has joined #jruby
<enebo>
kares: yeah so the problem seems to be that we need to const_missing on load but marshal.load should not
<enebo>
kares: My work-around I am trying is to only perform that in presence of Java objects (package/class)
subbu is now known as subbu|away
<kares>
enebo: makes sense, we likely should only need to rely on const_missing for Java types
<olleolleolle>
kares built it, enebo reviewed it, and I cheer-led
<enebo>
olleolleolle: just asked you guys about perf on the issue
shellac has quit [Ping timeout: 268 seconds]
<olleolleolle>
Apologies, I’ll click the logs link to re-read
<enebo>
olleolleolle: oh I literally just wrote it
<enebo>
olleolleolle: I do not know enough about puma to know where this object sets and how performance sensitive it may be but seeing that MRI has it as cext I think it might help
<enebo>
In some cases we can definitely do better as Ruby so it is difficult to know without someone running stuff
<olleolleolle>
(I’m mostly interested in our test suites being able to run without hitch.)
<enebo>
olleolleolle: yeah that is a good goal :P
<olleolleolle>
I don’t anticipate big changes in how it behaves. You both had some insight in a comment about massive IOBuffer + reuse of same. That that would remain troublesome.
<olleolleolle>
I’m still excited about this upcoming Puma release.
<enebo>
olleolleolle: the possibility of a big backing array is outstanding but that certainly was done to eliminate allocation so probably helps performance quite a bit
<olleolleolle>
Ah, trade-offs
KeyJoo has quit [Quit: KeyJoo]
olleolleolle has quit [Quit: olleolleolle]
subbu is now known as subbu|lunch
subbu|lunch is now known as subbu|shovelling
fredmorcos has quit [Remote host closed the connection]