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

Use shaded asm classes provided by Kryo. #175

Merged
merged 1 commit into from Mar 10, 2014
Merged

Conversation

pwendell
Copy link
Contributor

@pwendell pwendell commented Mar 7, 2014

Recent versions of Kryo have bumped the asm dependency to 4.X
which conflicts with many other libraries (including some Hadoop
versions) that use asm 3.X.

Presumably to help with this, Kryo now bundles a shaded version
of asm inside of its own jars. This patch alters chill to use
Kryo's shaded asm package.

If Kryo stops shading this in future versions or changes
the shaded package name, this can easily be changed
accordingly or reverted to vanilla asm.

Recent versions of Kryo have bumped the asm dependency to 4.X
which conflicts with many other libraries (including some Hadoop
versions) that use asm 3.X.

Presumably to help with this, Kryo now bunldes a shaded version
of asm inside of its own jars. This patch alters chill to use
Kryo's shaded asm package.

If Kryo stops shading this in future versions or changes
the shaded package name, this can easily be changed
accordingly or reverted to vanilla asm.
@ianoc
Copy link
Collaborator

ianoc commented Mar 10, 2014

ASM ends up bleeding into the API of kryo, makes this a very backwards incompatible change for us.

Is there a big win from this move?

(We attempted this before, #157 is the revert on that) We have services such a storm that incorporate kryo that makes it quite hard to integrate this sort of change.

@johnynek thoughts?

@pwendell
Copy link
Contributor Author

Hey @ianoc didn't know the history.

The win is that this enables use of chill for people who have older ASM versions on the classpath and can't easily isolate them from the use of chill. In particular the Hadoop clients depend on asm 3.X.

Could you explain in more detail the exact problem/conflict that arises? If chill is already pulling in Kryo's shaded asm then not sure how changing this reference could cause a problem... more specifics would be great.

@ianoc
Copy link
Collaborator

ianoc commented Mar 10, 2014

I think this is a degree of my bad/not enough coffee yet. I presumed you were bumping up to kryo's 2.23, which shades alot more classes. This appears to be entirely isolated to Chill changes. Don't really see any problems. Sorry will try drinking my coffee before replying in future. :)

ianoc added a commit that referenced this pull request Mar 10, 2014
Use shaded asm classes provided by Kryo.
@ianoc ianoc merged commit 1e70e90 into twitter:develop Mar 10, 2014
@pwendell
Copy link
Contributor Author

Thanks for merging!

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

2 participants