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

Added serialisation for SortedSet and ListSet. #152

Merged
merged 1 commit into from Nov 4, 2013
Merged

Added serialisation for SortedSet and ListSet. #152

merged 1 commit into from Nov 4, 2013

Conversation

stephanh
Copy link
Contributor

I added serialisation for immutable.SortedSet. I wanted to make it more generic but I couldn't figure out how to use CanBuildFrom and enforce the ordering. So any pointers on that would be welcome.

I threw in a fix for ListSet as well, since I was using it to test things.

Once this has been added, is it possible to propagate the fix to Scalding. I would like to use the CountMinSketch from Algebird which uses a SortedSet and currently that blows up.

val tRef = t.asInstanceOf[AnyRef]
kser.writeClassAndObject(out, tRef)
// After each intermediate object, flush
out.flush()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why flush after each object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because that is what Traversable does https://github.com/twitter/chill/blob/develop/chill-scala/src/main/scala/com/twitter/chill/Traversable.scala#L31.

I think @johnynek added Traversable maybe he can comment on why or if that is needed here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't flush, the Kryo output can overflow.

johnynek added a commit that referenced this pull request Nov 4, 2013
Added serialisation for SortedSet and ListSet.
@johnynek johnynek merged commit 3cd77a0 into twitter:develop Nov 4, 2013
@stephanh stephanh deleted the sortedset branch November 5, 2013 07:23
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

3 participants