Skip to content

SI-8519 collection.immutable.Map.apply is inefficient#4155

Merged
retronym merged 1 commit intoscala:2.12.xfrom
Ichoran:issue/8519
Jan 22, 2015
Merged

SI-8519 collection.immutable.Map.apply is inefficient#4155
retronym merged 1 commit intoscala:2.12.xfrom
Ichoran:issue/8519

Conversation

@Ichoran
Copy link
Contributor

@Ichoran Ichoran commented Nov 24, 2014

Added customized apply and contains methods for EmptyMap - Map4 and ListMap (both empty and Node).

@Ichoran
Copy link
Contributor Author

Ichoran commented Nov 24, 2014

Note: JIRA didn't let me log in, so I haven't linked to this PR from there.

@scala-jenkins scala-jenkins added this to the 2.12.0-M1 milestone Nov 24, 2014
@adriaanm adriaanm self-assigned this Nov 24, 2014
@Ichoran
Copy link
Contributor Author

Ichoran commented Nov 25, 2014

The test passes on my machine...not sure what the issue is?

@Ichoran
Copy link
Contributor Author

Ichoran commented Nov 25, 2014

PLS REBUILD/pr-scala@693b0647f86e9702ac21e87f5e093346bf2d6edd

@scala-jenkins
Copy link

(kitty-note-to-self: ignore 64290201)
🐱 Roger! Rebuilding pr-scala for 693b064. 🚨

@retronym
Copy link
Member

fail - run/t8549.scala
...
java.io.InvalidClassException: scala.collection.immutable.ListMap$EmptyListMap$; local class incompatible: stream classdesc serialVersionUID = -8256686706655863282, local class serialVersionUID = 307923064409500379

Your change might have changed the implied serialVersionUID of the module class EmptyListMap$ . Perhaps this was only under -optimize, which might explain why if failed on Jenkins but not for you.

We might be able to retrofit @serialVerionUID(-8256686706655863282L) on that object to make things work.

I think the principled approach is to actually use the serialization proxy pattern to avoid serializing these MapN / SetN / ... in the first place.

To help debugging, here's how to extract the SerialVersionUID (calculated or explicitly annotated) of a class programatically:
f9abdce#diff-899f0b1feda908d4ef15744fae0fb8dbR9

@Ichoran
Copy link
Contributor Author

Ichoran commented Nov 25, 2014

@retronym - Thanks, I should have looked into it more deeply to see why it was failing. (Not the first time I have forgotten the difference between -optimize and not.) I guess we're trying to be serialization compatible in 2.12, so the thing to do for now would be to apply the annotation.

Added customized apply and contains methods for EmptyMap, Map1 - Map4 and ListMap (both EmptyListMap and Node).

The modifications appear to be able to (sometimes) change the SerialVersionUID of EmptyListMap, so that has been fixed at the old value.
@lrytz
Copy link
Member

lrytz commented Dec 4, 2014

To help debugging, here's how to extract the SerialVersionUID (calculated or explicitly annotated) of a class programatically:
f9abdce#diff-899f0b1feda908d4ef15744fae0fb8dbR9

You can also use the serialver command line tool

Copy link
Member

Choose a reason for hiding this comment

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

could eliminate the excess newline

@lrytz
Copy link
Member

lrytz commented Dec 4, 2014

LGTM, otherwise

@retronym
Copy link
Member

LGTM, too.

retronym added a commit that referenced this pull request Jan 22, 2015
SI-8519  collection.immutable.Map.apply is inefficient
@retronym retronym merged commit b9c9323 into scala:2.12.x Jan 22, 2015
@adriaanm adriaanm added the 2.12 label Oct 29, 2016
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.

5 participants