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

Cleanup in aisle patmat #5612

Merged
merged 3 commits into from Jan 4, 2017
Merged

Cleanup in aisle patmat #5612

merged 3 commits into from Jan 4, 2017

Conversation

adriaanm
Copy link
Contributor

A backport and a few cleanups to desperately try to combat scala/scala-dev#251

adriaanm and others added 3 commits December 21, 2016 16:08
…ckport]

Hash consing of trees within pattern match analysis was broken, and
considered `x1.foo#1` to be the same tree as `x1.foo#2`, even though
the two `foo`-s referred to different symbols.

The hash consing was based on `Tree#correspondsStructure`, but the
predicate in that function cannot veto correspondance, it can only
supplement the default structural comparison.

I've instead created a custom tree comparison method for use in
the pattern matcher that handles the tree shapes that we use.

(cherry picked from commit 79a52e6)
@scala-jenkins scala-jenkins added this to the 2.11.9 milestone Dec 22, 2016
@retronym
Copy link
Member

retronym commented Dec 22, 2016

Trying out a restarr on the first commit in https://scala-ci.typesafe.com/job/scala-2.11.x-validate-publish-core/2992 and subsequent rebuilds.

All green for those builds, whereas trying 2.11.x immediately walked into the infamous failure:

image

Hypotheses: your change has:

  • fixed a subtle and intermittent logic error
  • worked around a bug in our code generation
    • ... which might be interplaying with a JVM bug.

To start testing out the last point, I tried disabling the C2 JIT compiler in the SBT process. I still experienced the failure: https://scala-ci.typesafe.com/job/scala-2.11.x-validate-publish-core/3003/console

@lrytz
Copy link
Member

lrytz commented Dec 22, 2016

@retronym does -XX:TieredStopAtLevel=2 work with JDK 6? I thought 6 still has server / client, tiered compilation came with 7.

@lrytz
Copy link
Member

lrytz commented Dec 22, 2016

I also played a bit with jvm flags yesterday, but using 8. lrytz@4448e22. i did two runs, but the failure didn't show up.

@retronym
Copy link
Member

Hmm, maybe the SBT runner on Jenkins was ignoring -J-XX:...?

@lrytz
Copy link
Member

lrytz commented Dec 22, 2016

with java 6, java -XX:TieredStopAtLevel=2 is OK, but java -XX:AAATieredStopAtLevel=2 gives Unrecognized VM option 'AAATieredStopAtLevel=2' - so the setting is accepted, not sure what it does.

@adriaanm
Copy link
Contributor Author

I say we bury this bug along with 2016. Lets merge this one when green enough, and restarr in the new year. We'll have three weeks of battle testing for the remainder of the 2.11.9 cycle.

@adriaanm
Copy link
Contributor Author

/synch

@adriaanm
Copy link
Contributor Author

/rebuild

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants