shellac has quit [Quit: Computer has gone to sleep.]
<kares>
headius: enebo any reason why joda time wasn't updated to latest 2.9 and stayed at 2.8.2 ?
rdubya has joined #jruby
drbobbeaty has joined #jruby
olle has joined #jruby
bbrowning_away is now known as bbrowning
<nirvdrum>
flavorjones: Unfortunately, I don't believe anyone from the JRuby team manages that Dockerfile. It looks like cpuguy83 is still handling the updates.
<nirvdrum>
I'm sure that could be worked out though.
<nirvdrum>
flavorjones: I'm curious how you test with Docker though. I looked at your .travis.yml and nothing immediately stood out to me.
<kares>
guess the only think blocking JODA update might be:
<kares>
Fixed to handle JDK 8u60 [#288, #291] Without this fix, formatting a time-zone will print "+00:00" instead of "GMT" for the GMT time-zone
<kares>
... this is from the change-log for 2.9
<nirvdrum>
kares: If JRuby has dropped support for Java 7, why not move to the JDK 8 date and time API?
<nirvdrum>
I think we experienced one regression from joda. It's unfortunate, but dropping a dependency was really nice.
<kares>
nirvdrum: hey! cause its not a green field
<nirvdrum>
What's not? JRuby?
<kares>
yeah it would be nice but it needs to get properly deprecated as its part of the API at this point
<nirvdrum>
Ahh. Bummer.
<nirvdrum>
The API itself is quite similar. I think JDK 8's date & time is basically a vendored version of joda anyway.
<kares>
yep it is and I would certainly love to switch :)
<nirvdrum>
You'd know much better than I would. But what does JRuby expose that would need to be deprecated?
<kares>
but that's not the way if you have org.joda.time.DateTime on JRuby's API around
<nirvdrum>
Ahh. Arguments?
<kares>
think its pretty much RubyTime.java
<nirvdrum>
lopex: Are you around?
<kares>
except for that hopefully not much - Date/DateTime internals also use JODA but I doubt any-one is using that to extract the native parts
<kares>
except for AR-JDBC :)
shellac has joined #jruby
rrutkowski has joined #jruby
rrutkowski has quit [Client Quit]
rrutkowski has joined #jruby
adam12 is now known as adam
adam is now known as Guest68570
mkristian has quit [Quit: This computer has gone to sleep]
mkristian has joined #jruby
mkristian has quit [Quit: This computer has gone to sleep]
projectodd-ci has joined #jruby
mkristian has joined #jruby
rrutkowski has quit [Quit: rrutkowski]
rrutkowski has joined #jruby
Guest68570 is now known as adam12
rrutkowski has quit [Quit: rrutkowski]
mkristian has quit [Quit: This computer has gone to sleep]
<flavorjones>
nirvdrum: I don't pay attention to Travis. I use Concourse for testing: https://ci.nokogiri.org/
<flavorjones>
Concourse build jobs can use Docker images as the base layer, which we use for JRuby 1.7 and 9.1 testing.
<flavorjones>
I'm actually happy to help generate Docker images for JRuby based on green builds, but didn't want to either a) reinvent the wheels, or b) duplicate effort
olle has quit [Quit: olle]
lanceball has quit [Excess Flood]
lanceball has joined #jruby
<headius>
flavorjones: we don't have a lot of cycles to maintain packages and images for every platform but if docker's easy to maintain we could make it part of release process
<headius>
I have no idea wht's involved
<headius>
nirvdrum, kares: I think I've seen a handful of cases where someone's pulling Joda DateTime out of RubyTime in extensions but I can't recall them right now
<headius>
because we integrate more closely to Java and have lots of active users we have to ba cautious about changing dependencies
<headius>
just shading jline into our readline library broke an ext
<headius>
users suck
<headius>
enebo: we should chat about what needs to happen for 9.2
<headius>
getting 2.5 stuff stable and "mostly" complete is in there...symbol love is in there
<headius>
windows native process should be in there...and possibly shoring up native IO on Windows
<enebo>
yeah and we should be done by beginning of May (latest) since we are going to Poland and RubyKaigi at end of may
<enebo>
Not an unreasonable amount of time to get all the above done
<headius>
beginning of May is pretty good
<headius>
that gives us some time to shore up JRuby wrt Discourse and anything else we manage to get running
<enebo>
yeah
xardion has joined #jruby
mistergibson has quit [Quit: Leaving]
shellac has quit [Remote host closed the connection]
mistergibson has joined #jruby
GitHub37 has joined #jruby
GitHub37 has left #jruby [#jruby]
<GitHub37>
[jcodings] nirvdrum opened pull request #21: Reduce the number of individual InputStream#read calls by reading in batches. (master...more-efficient-array-reader) https://git.io/vAKID
<nirvdrum>
headius, enebo: Feel free to modify https://github.com/jruby/jcodings/pull/21 as you see fit. I added a rather lengthy comment in there. And ArrayReader#readIntArray could maybe be done more efficiently.
<nirvdrum>
As for motivation, you need to look at ZipFileInputStream to see how nutty this all is.
<headius>
nirvdrum: so this reads everything into memory now before unzip stream?
<nirvdrum>
headius: It improves creating transcoder time.
<headius>
why wouldn't we just use new DataInputStream(new BufferedInputStream(dis))?
<headius>
or do that in openStream I guess
<headius>
rather than just returning a DIS wrapping the jar stream
<nirvdrum>
That could be fine, too. You'll just end up reading each time the buffer depletes, I think.
<nirvdrum>
The other sucky part is the JAR file entry reading scales either with the size of the JAR or the number of entries. So, while we have the same code as you do, it takes us ~2s to load some of these files. With this PR that drops to ~100ms.
<nirvdrum>
I'll see what the time looks like with a BufferedInputStream.
<headius>
thank you
<headius>
I don't understand...why does it take 2s for you to load these files?
<headius>
this all gets mixed into JRuby jar which has thousands of entries
<nirvdrum>
I don't entirely know why. But our JAR is roughly twice the size.
<headius>
wow
<nirvdrum>
But I think we have a lot more entries.
shellac has joined #jruby
<nirvdrum>
Yeah. I couldn't believe creating a transcoder was taking this long. Then I started profiling and saw reading this input stream incurs allocating a singe-byte array and a zip file decode for each byte read.
<nirvdrum>
Reading into a buffer instead was considerably faster.
<nirvdrum>
But I didn't want to duplicate the work of DataInputStream.
<headius>
yeah I suspect just having a buffered stream in there would be sufficient
<nirvdrum>
I'll take a look. This was the rare case where we could get the entire input stream size easily, so I was trying to take advantage of that.
<nirvdrum>
But code cleanliness is nice, too.
<headius>
yeah if this were general-purpose code it would be more of a concern, reading it all into memory
<headius>
but for this it's not a huge deal
<headius>
I just figured it would be simpler
<nirvdrum>
I'll give you guys a bit of a boost as well.
<nirvdrum>
I think more importantly it won't vary as wildly as your JAR grows or shrinks.
<nirvdrum>
The real problem I was seeing was the sheer number of syscalls affecting VM performance. I was seeing upwards of 15s to load some of these in a VM.
<kares>
headius: regarding discourse I wanted to use it for testing out Postgres with AR-JDBC
<kares>
you did have a RubyString implements CharSequence prototype
<kares>
not sure if its coming or not, although guess you can pass ByteList for single-byte encodings
<headius>
nirvdrum: that's crazy
<headius>
kares: yeah it's possible to fix the read side I think but then it dumps into a StringBuilder to produce the output
<headius>
either we would want to fork this library and make it work with something like ByteList, or we accept the transcoding and object overhead
<headius>
I also have no idea how complete it is wrt rinku API
<headius>
rinku is pretty small and only exposes one function
<headius>
kares: pg + ar-jdbc improvement would be great to drive from Discourse
<headius>
I'm also thinking jruby+discourse has contracting/hosting potential too, bring Discourse to Java shops
shellac has quit [Quit: Computer has gone to sleep.]
<headius>
especially if we can figure out how to war the sucker up :-)
<kares>
certainly its pbly the best candidate for any Rails perf work
<headius>
there's at least one Rails benchmark that's based on Discourse right now
<nirvdrum>
So that does the single-byte allocation and then calls read with an array. And that read call re-reads the zip file from what I can tell.
<nirvdrum>
I didn't trace the native code, but it seems unlikely that it'd hold on to some internal buffer.
<nirvdrum>
lopex: readInt against an in-memory stream is pretty cheap. Sure, you're doing N calls, but as long as it's just doing a byte read (without this crazy byte[] allocation), I wouldn't worry about it.
<nirvdrum>
Although I wonder if donv's issue could have been resolved by making the table reads faster.
<headius>
there's also a read that takes byte[]
<nirvdrum>
Yes.
<nirvdrum>
DataInputStream#readFully just calls that
<lopex>
yeah we could compose the ints ourselves
<headius>
nirvdrum: ok
<nirvdrum>
BufferedInputStream does too, I believe.
<nirvdrum>
But, yeah, the basic point is to call read with a byte[] of size > 1 when dealing with ZipFileInputStream.
<nirvdrum>
I'm happy to update the PR with whatever you guys want. I'm also happy to close it out entirely in favor of something you want to write. But it's illustrative of the code region I think we can improve.
<headius>
nirvdrum: does buffered stream perform the same as fully reading?
<nirvdrum>
Getting there. I was just eating a late lunch.
<headius>
oh ok, that's all I'm wondering about :-)
<lopex>
headius: internal buffer defaults to 8192
<lopex>
and fill does all the magic
<lopex>
headius: it read in chunks and doubles that
<lopex>
*reads
<lopex>
read(buffer, pos, buffer.length - pos) just that
<headius>
sure
<lopex>
on input stream
<lopex>
but ideally we'd read the byte array and then compose ints from that
<lopex>
if you want to minimize calls
<lopex>
and also we dont care about concurrent reads here right ?
<nirvdrum>
BufferedInputStream looks good.
<nirvdrum>
Maybe 10 - 20 ms slower, but that's nearly impossible to account for. Measurement error is high for disk activity.
<headius>
lopex: but we're still talking load time right?
<nirvdrum>
lopex: I'm not really concerned with the number of calls to DataInputStream if they're hitting something in memory.
<headius>
once we have the ints we aren't re-processing the bytes again
<headius>
nirvdrum: yeah exactly
<headius>
buffered stream seems simplest and still good perf, so I'd say go with that
<lopex>
yeah, but bigger reads are always better
<lopex>
BIS takes the buffer size as a parameter there
<nirvdrum>
lopex: That's why I wrote it the way I did. But I have to admit a 1 line change is really appealing.
<nirvdrum>
And BufferedInputStream at least attempts to perform aligned reads.
<lopex>
what does available actually do internally ?
<nirvdrum>
The other area to potentially optimize is jcodings will read the same file multiple times for multiple transcoders. In reality, you're probably only going to encounter that during test.
<headius>
that should be pretty easy to cache somewhere
subbu|lunch is now known as subbu
<nirvdrum>
headius: Do you want to commit that one line change directly? Otherwise, I can update the PR.
<headius>
yeah just go for it
<nirvdrum>
I updated the PR since I already had it open.
<headius>
ok
GitHub150 has joined #jruby
GitHub127 has joined #jruby
<GitHub150>
[jcodings] headius closed pull request #21: Reduce the number of individual InputStream#read calls by reading in batches. (master...more-efficient-array-reader) https://git.io/vAKID
GitHub150 has left #jruby [#jruby]
<GitHub127>
[jcodings] headius pushed 2 new commits to master: https://git.io/vAK0g
<GitHub127>
jcodings/master 437dfb3 Kevin Menard: Buffer input stream reads in ArrayReader to avoid cost of many single-byte reads....
GitHub127 has left #jruby [#jruby]
<GitHub127>
jcodings/master 9f28476 Charles Oliver Nutter: Merge pull request #21 from nirvdrum/more-efficient-array-reader...
<nirvdrum>
I don't think I have write permissions to that repo anyway. I was really asking if you just wanted to commit the change since it was so small. But, we're all good now.
<nirvdrum>
Thanks!
<headius>
one of us will flip a release this week
<headius>
lopex: anything else pending for jcodings?
<headius>
enebo: hey, so I'm looking at this persistent JSR-223 variables thing
<headius>
the default behavior right now is "global" so when you say engine.put(name, value) it goes into "$name" in Ruby and you have to access it that way
<headius>
if we use persistent mode, then it goes into "name" and can be accessed like a local and local variables also are persistent like in other 223 engines
<headius>
but then $name is not there anymore
<headius>
so there's one test that fails with persistent, and it's fixed by removing the $
<enebo>
headius: tbh I don't care but I was fairly certain the reporters assertion used to not be true
<headius>
which assertion?
<nirvdrum>
No problem. To update jcodings I need to update joni, too. We're a couple releases behind.
<headius>
that the variables used to be persistent?
<enebo>
headius: that other impls persist to locals across calls
<headius>
well, his example shows they do though
<headius>
he had a big test file where he tries the same operations against local vars in each impl
<enebo>
headius: yeah I am just saying I do not think it used to be that way
<headius>
that I don't know
<enebo>
headius: the same guy made most of the impls initially from Sun
<enebo>
sundar.... something
<headius>
it's possible some engines did it like us at some point and were "fixed"
<headius>
yeah Sundar
<enebo>
yeah so I think likely it has changed
<headius>
but I don't think he's worked on that in a long time
<headius>
so for 9.2 this seems like it's probably ok
<headius>
223 is not heavily used (at least for us, as far as we know) and this can still be tweaked back to global via a property
<enebo>
I am happy with it for 9.2 but less happy with 9.1
<headius>
I think this mimics what the other engines to better
<headius>
to/do
<enebo>
since if anyone tries to hit $goo it will break
<headius>
agree
<enebo>
but yeah I don't care for 9.2 if it matches better than great
<headius>
I will make the change on master then and update the test
<enebo>
ok
<enebo>
yeah I find 223 to be a bit of a failure overall but if we can make it more useful than :)