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

Truffle pr zlib #3515

Merged
merged 2 commits into from
Dec 4, 2015
Merged

Truffle pr zlib #3515

merged 2 commits into from
Dec 4, 2015

Conversation

chrisseaton
Copy link
Contributor

@eregon @nirvdrum @pitr-ch please review (it's all imported code though, so no point commenting on the actual code).

@chrisseaton
Copy link
Contributor Author

For something simple like adler32 this is about 200x slower than using Java djberg96/pr-zlib#3 (comment), and I would imagine that gets worse for less tight code, so this is perhaps not the final code we'll use for zlib. However the functionality seems almost perfect for now.

@nirvdrum
Copy link
Contributor

nirvdrum commented Dec 4, 2015

It looks good to me. It's surprising the imported lib gets everything but CRC32 right, though.

@eregon
Copy link
Member

eregon commented Dec 4, 2015

Could we keep the few methods we already implement?
Particularly the highly numeric ones?

@chrisseaton
Copy link
Contributor Author

Could do. I removed them as they aren't being used in anything high performance yet and I felt like less code to maintain was better until we did. What does everyone else think? Of course the Java code is still there if we want it now, or in the future.

@chrisseaton
Copy link
Contributor Author

I'll merge this as is. The Java equivalents are not friendly to AOT, so removing them for now simplifies both our Java codebase and the SVM substitutions. We can put them back as soon as we see a high-performance use case.

chrisseaton added a commit that referenced this pull request Dec 4, 2015
@chrisseaton chrisseaton merged commit 5c5b5df into master Dec 4, 2015
@chrisseaton chrisseaton added this to the truffle-dev milestone Dec 4, 2015
@chrisseaton chrisseaton self-assigned this Dec 4, 2015
@chrisseaton chrisseaton deleted the truffle-pr-zlib branch December 4, 2015 21:44
@pitr-ch
Copy link
Member

pitr-ch commented Dec 9, 2015

@chrisseaton did you have time to investigate why is it that much slower? Bad algorithms or we do not optimise it well?

@chrisseaton
Copy link
Contributor Author

I haven't looked into it in any depth - but I just wanted it for the completeness rather than the performance for now.

@enebo enebo added this to the Non-Release milestone Dec 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants