Skip to content
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

Zlib crc improvements #1805

Merged
merged 3 commits into from Jul 16, 2014

Conversation

Projects
None yet
2 participants
@grddev
Copy link
Contributor

grddev commented Jul 11, 2014

The main change here is that CRC32Ext and Adler32Ext were replaced/inlined into RubyZlib. The abstractions are not used elsewhere in the code, and considering the obvious copy-paste errors in CRC32Ext, I'm guessing they haven't been used outside RubyZlib.

For Adler32, the reflection trick previously used was replaced with a call to adler32_combine, which has reasonable performance (and does not rely on accessing a private field).

For CRC32, we still (conditionally) use the reflection trick for optimum performance as using crc32_combine completely dominates the runtime for anything but really long byte lists. Compared to the previous version, we at least fall back on a slower solution in the unlikely even that the private variable name would change in a future release.

For both cases, this change optimises the normal case (with default starting checksums) to not perform any additional computation. Thus, the normal case neither requires reflection, nor slow calls to combine.

While changing the RubyZlib class, the TODO regarding using the CRC-32 table from JZlib was resolved as well.

grddev added some commits Jul 11, 2014

Inline Adler32Ext
As an improvement, the new implementation does not rely on reflection
to adjust for a starting checksum. Instead, it uses adler32_combine, as
that computation is reasonably cheap anyhow.

Compared to Adler32Ext, the adjustment is also only performed when a
starting checksum other than the default is provided.
Inline CRC32Ext
Compared to the inlined version of Adler32, this one unfortunately still
relies on reflection for adjusting the starting checksum. This is due
to the fact that crc32_combine is much more expensive, and completely
dominates the runtime up to quite long byte lists.

However, in the event that the reflection trick stops working, we now
fall back to the slower approach, ensuring compatibility with an
unlikely future Java release where the internal CRC32 field name is
changed.

Compared to CRC32Ext, however, neither the reflection nor the slow path
is used when the starting checksum matches the default.

enebo added a commit that referenced this pull request Jul 16, 2014

@enebo enebo merged commit 9f947be into jruby:jruby-1_7 Jul 16, 2014

1 check failed

continuous-integration/travis-ci The Travis CI build could not complete due to an error
Details

@enebo enebo added this to the JRuby 1.7.14 milestone Jul 16, 2014

@enebo enebo modified the milestones: JRuby 1.7.14, JRuby 1.7.15 Aug 27, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.