-
-
Notifications
You must be signed in to change notification settings - Fork 925
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use com.jcraft.jzlib.JZlib instead of hacking the internals of java.util.zip.CRC32 #5088
Conversation
Now that I got the results deciphered, it seems that there's actually a problem there in I'll look into this. |
@jmiettinen Any luck? |
I'll check this out during easter holidays this weekend. |
Not a super time-critical item so don't sweat it 😀 Looking at 9.2 release in about a month. |
Is there a way to run the build again as I don't see my changes stalling the build totally? |
I restarted the whole thing. But when did you pull this from master? We haven't tagged off or completed the remaining 2.5 failures, so that could be the source of much of this. The hangs are unfortunately elusive...something concurrency-related in MRI's tests that we stumble over. |
I probably should've started from some known-to-work tag instead of just branching from whatever happened to be the master at that time (334a7f0). Should I rebase this on some known-to-work-tag and force-push, or what's the preferred way of working here? |
@jmiettinen Rebasing against current master is probably your best bet. Do that and repush and we'll confirm that the failures match expected. I'm working today to try to get master properly green, also. |
…at's the way to simulate 32-bit unsigned ints in the JVM
I rebased this on master that had a passing Travis build. Now I get these errors:
Any ideas about these? I can't really see any connection between that I've done and reading jars. @headius |
That has had some failing builds due to another PR that tried to fix some aspects of symlink traversal during load. It is probably not you. What reg did you branch from? I'm fine merging this. |
I rebased this on e7bf9d1 which has a passing build: https://travis-ci.org/jruby/jruby/builds/367136555. Also, there does not seem to be any connection between CRC32 and the JAR-tests. That's why I am a bit confused here. |
due some local issues with Java 8 I'm now running under 9 and hit this 3 liner warning. |
vefiried and landed manually as: f1f2c72 |
The code the seems to be a remnant of the time before
JZLib
and uses internals ofjava.util.zip.CRC32
when same thing could be achieved throughJZLib
.This can cause a performance regression as methods in
CRC32
use intrinsics, but I would not expect that to be major issue as running a lot ofCRC32
does not sound like the ideal use case for JRuby.This should remove one invasive access warning from #4834