nirvdrum has joined #jruby
nirvdrum has quit [Read error: Connection timed out]
nirvdrum has joined #jruby
lucasb has quit [Quit: Connection closed for inactivity]
nirvdrum has quit [Ping timeout: 240 seconds]
Antiarc_ has quit [Ping timeout: 276 seconds]
Antiarc has joined #jruby
Antiarc has quit [Read error: Connection reset by peer]
Antiarc has joined #jruby
Antiarc has quit [Ping timeout: 240 seconds]
Antiarc has joined #jruby
Antiarc has quit [Quit: ZNC 1.7.2+deb2ubuntu0.1 - https://znc.in]
Antiarc has joined #jruby
drbobbeaty has quit [Ping timeout: 250 seconds]
shellac has joined #jruby
sagax has quit [Ping timeout: 245 seconds]
drbobbeaty has joined #jruby
sagax has joined #jruby
_whitelogger has joined #jruby
shellac has quit [Quit: Computer has gone to sleep.]
shellac has joined #jruby
nirvdrum has joined #jruby
lucasb has joined #jruby
<rdubya[m]> enebo: kares I think I have the mssql gem in a good place, I created a placeholder PR and assigned both of you as reviewers, right now the travis build is failing because 50.5 hasn't been set up in the arjdbc 50-stable branch but I think once that happens we should be good
shellac has quit [Quit: Computer has gone to sleep.]
<kares[m]> nice!
<kares[m]> enebo: nice on the cast issue repro - wanted to try smt simple as well!
<kares[m]> wonder if there's another way, now that we reproduce, avoiding the lock ...
<kares[m]> JRuby could use a test target (optional) to test code like that (with 'real' gems)
shellac has joined #jruby
<enebo[m]> kares: I do wonder how big a deal this really is? I meam how many types actually exist in any ordinary app
<enebo[m]> kares: 20,000 types during startup having a synch block should not be a huge impact and many get loaded before thread listening stuff tends to happen
<enebo[m]> kares: Trying to think of something more esoteric like constantly making new types (ala DCI pattern)
<enebo[m]> although that pattern is already slow as hell
<enebo[m]> kares: what was the issue with lock contention in AR you found and fixed recently?
xardion has quit [Remote host closed the connection]
rusk` has quit [Remote host closed the connection]
xardion has joined #jruby
shellac has quit [Ping timeout: 240 seconds]
<kares[m]> okay, that logic makes sense ... just a bit precautious after seeing some sync issues
<kares[m]> that fix was on (singleton) class generation at runtime (due `obj.clone`)
<kares[m]> did not consider type stability since the report seemed like the cast exception (ivar shape generation) happened under load some time into running sidekiq
<enebo[m]> kares: I believe this is only happening because the threads happen to be loading the class at the same time so it is only happening at the beginning....BUT...the error may take longer to blow up because not all those ivars may be hit until later while it is running
<enebo[m]> but I am mildly worried there is some usecase with lots of loaded classes ... OTOH those use cases would never do well for any impl
<headius[m]> enebo: kares so we are ok with the synchronization then
<kares[m]> fine by me
<headius[m]> like I mention in the PR this will eventually be moot once individual objects can have different shapes, but I don't have a timeline for implementing that
<enebo> headius[m]: yeah my only comment is really about impact of continuous class creation (which maybe is some form of singleton) but I doubt those would do well anyways
<enebo> headius[m]: but my overall analysis is most types load before threads fire up and number of types overall is relatively small compared to objects made from them
<enebo> seems like at most it could have a very small impact on startup
<headius[m]> shouldn't affect singletons...the variable layout is calculated and stored on the nearest real class
<enebo> ok so much the better then
<headius[m]> and singletons are already constructed by the time they become singleton
<headius[m]> so it should be fine
<enebo> ah yeah that makes sense
<headius[m]> I'll merge in the PR then
<enebo> so then the only case would be some weird thing where people continuously make classes/modules and I feel that is pretty out of scope as a worry
<enebo> yeah you saw my repro script?
<headius[m]> yeah
<headius[m]> my confusion at this point is how two threads would come to different conclusions about the shape of the object
<headius[m]> it's just walking the same set of classes and methods
<enebo> headius[m]: yeah that I don't get either...but I did see 6/7 and 4/7 on shapes too
<headius[m]> maybe we can get this repro to work without this library
<enebo> headius[m]: so whatever that race is perhaps is setting some state on an intermediate object so next thing skips it
<enebo> we walk down to object and mark each object as done or something?
<headius[m]> hmm
<enebo> I thought there was something marked but I don't see it in the before version either
<headius[m]> right
<headius[m]> there's nothing marked for this
<headius[m]> we did sort of mark for the old become_java reification but that's not related to this code
<headius[m]> e.g. it reified from the top down
<enebo> if (cls instanceof RubyClass) {
<enebo> cls = ((RubyClass)cls).getSuperClass();
<enebo> oh cls!=null is around that
<enebo> I was trying to decipher how nested modules worked
<enebo> this is original discoverInstanceVariables
<headius[m]> yeah
<enebo> Yeah I would definitely like to understand why now
<enebo> I mean sync'ing definitely is a valid fix for 9.2.9 if we do not shed more light but this is a pretty weird issue
<headius[m]> it doesn't seem like it should happen, does it
<enebo> Well if you think about ways modules can be included I could see some races
<enebo> like self.included doing include/extend
<enebo> or something like that
<headius[m]> yeah if they were being included later
<enebo> but concurrently both doing that
<headius[m]> but this case doesn't seem to do that
<enebo> but I do not see that here
<enebo> so perhaps that scenario does justify the sync regardless but I still want to know why this case is broken
<enebo> It is down to a new on one class in that library
<enebo> Also I noted that execution speed of instrs themselves helps the race if it executes slower
<enebo> That does point more towards an 'self.included' sort of race
<headius[m]> so regarding the overhead of the sync...it will probably be lower than the overhead of generating the class twice, which would have been happening sometimes under contention before
<headius[m]> yeah I just don't see any weirdness in these types in that http-cookie library
<enebo> yeah if it is actually happening much
<enebo> yeah I guess if we can think of a case like dynamic include it means we have to have a sync or accept some double creation with checks sort of soln
<headius[m]> there's an inherited hook but it doesn't do anything to the types
<enebo> sync overhead here should not be a perf problem since type creation is not a major perf overhead in an app
<enebo> you will realize in an hour that class_to_symbol does affect it :)
<enebo> This is largely a joke but I did not verify how much of a joke that is
<enebo> or @@class_map.[] impl :)
<headius[m]> hmmm
<headius[m]> that could do it
<headius[m]> so first thread jumps in and goes with an initial size from inspecting methods...constructs object and runs this initialize code
<headius[m]> second thread already in process of calculating size sees the later fields
subbu is now known as subbu|lunch
<enebo> does instance_variable_set walk down and find existing ivars or will it make it on obj
<enebo> I would think it would hope to find it first and only create on top obj in case it is not there
<headius[m]> each class manages its variable list independently
<headius[m]> only classes that get instantiated will ever have a variable table
<enebo> oh that is on the class
<headius[m]> but as far as I can tell I'm not looking at the existing variables in that table so still not sure if this could do it
<headius[m]> I still can't get an independent repro
<enebo> you mean more reduced that calling that single constructor
<enebo> I guess you could just copy that code into a file and keep deleting crap
<headius[m]> reduced as in not needing that library
<enebo> This has to be something which is just being obfuscated by amount of extra code around it perhaps
<headius[m]> yeah I'm trying to simulate it
<headius[m]> does not fail for me so far
<enebo> you running in interp too?
<headius[m]> yeah
<headius[m]> does it repro for you?
<enebo> Only other thought on that is more stuff is executed as part of library so it may expand the race window
<headius[m]> yeah possible
<headius[m]> something we're not seeing
<enebo> It still fails the what is the theory bit here
<enebo> Observationally I could not get the original library to fail if it was the main file and got JITd
<enebo> from that I can say I think execution speed is a function of how large the race window is
<enebo> OR interpretation somehow is the cause but I don't really get that
<headius[m]> your script fails immediately for me
<enebo> yeah first 2 threads probably
<headius[m]> ok I realized I have logging in there
<enebo> the 200 was left over from me not understanding
<headius[m]> so I dumped the variables they fine
<headius[m]> I see both of these:
<headius[m]> ```
<enebo> but it is not 100% failing right you have to run it a few times
<headius[m]> oops
<enebo> neat
<enebo> so that is fascinating
<headius[m]> the missing vars are from HashStore itself
<enebo> @jar and @gc_index is literally in initialize from HashStore
<enebo> How could that happen? :)
<headius[m]> yeah exactly
<headius[m]> that's strange
<enebo> It is like somehow it broke out of the introspection early
<enebo> which somewhat was my original posit that some type state changes and it stops/aborts/doesnotlookat
<enebo> but what is that? :P
<enebo> invalidation of a type somehow
<headius[m]> yeah I don't know
<headius[m]> hmm
<headius[m]> full output is like this, and then it starts to class cast error
<headius[m]> kares: enebo I am addressing issues in that PR and then will merge
<enebo> ok
<headius[m]> bizarre
<headius[m]> with the patch in place I only ever see one shape print out but it's still vacillating between the two!
subbu|lunch is now known as subbu
jrafanie has joined #jruby
nirvdrum has quit [Ping timeout: 245 seconds]
drbobbeaty has quit [Quit: My MacBook Pro has gone to sleep. ZZZzzz…]
nirvdrum has joined #jruby
jrafanie has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]
drbobbeaty has joined #jruby
nirvdrum has quit [Ping timeout: 265 seconds]
<headius[m]> I think enebo and I figured out the variability
<headius[m]> we lazily compile methods, so they are just in AST form until first called
<headius[m]> so all the methods in HashStore were still AST and did not properly scan for instance variables
<headius[m]> This actually could affect many other methods, so we're probably not right-sizing objects for most cases
lucasb has quit [Quit: Connection closed for inactivity]