Skip to content

Fix #3965: Trying to make higher-kinded equality correct and efficient [WIP] #3975

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

Closed
wants to merge 8 commits into from

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Feb 8, 2018

#3970 fixed equality for higher-kinded types but came at a steep performance penalty. This PR is trying to avoid the penalty.

First, since correct hashing under binders seems to be very expensive (see performance
data for #3970), let's try have fewer types that require this.

Since correct hashing under binders seems to be very expensive (see performance
data for scala#3970), let's try have fewer types that require this.
@odersky odersky changed the title Fix #3965:Trying to make higher-kinded equality correct and efficient [WIP] Fix #3965: Trying to make higher-kinded equality correct and efficient [WIP] Feb 8, 2018
@odersky
Copy link
Contributor Author

odersky commented Feb 8, 2018

test performance please

@dottybot
Copy link
Member

dottybot commented Feb 8, 2018

performance test scheduled: 1 job(s) in queue, 0 running.

@odersky
Copy link
Contributor Author

odersky commented Feb 8, 2018

test performance please

@dottybot
Copy link
Member

dottybot commented Feb 8, 2018

performance test scheduled: 1 job(s) in queue, 1 running.

@dottybot
Copy link
Member

dottybot commented Feb 8, 2018

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/3975/ to see the changes.

Benchmarks is based on merging with master (39980f3)

1 similar comment
@dottybot
Copy link
Member

dottybot commented Feb 8, 2018

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/3975/ to see the changes.

Benchmarks is based on merging with master (39980f3)

@odersky
Copy link
Contributor Author

odersky commented Feb 8, 2018

test performance please

@dottybot
Copy link
Member

dottybot commented Feb 8, 2018

performance test scheduled: 1 job(s) in queue, 0 running.

Rework hashing and equality so that two isomorphic types are identified
even if they are dependent (i.e. have back edges from a BoundType such
as ParamRef or RecThis to its HKLambda or RecType

Method and PolyTypes are still generative.

This fixes scala#3965
@odersky
Copy link
Contributor Author

odersky commented Feb 8, 2018

test performance please

@dottybot
Copy link
Member

dottybot commented Feb 8, 2018

performance test scheduled: 1 job(s) in queue, 1 running.

@dottybot
Copy link
Member

dottybot commented Feb 8, 2018

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/3975/ to see the changes.

Benchmarks is based on merging with master (39980f3)

Before hashing with a non-null binders list, check whether
the type has a stable hash. Hash-stability results are cached in
NamedTypes and AppliedTypes.
@odersky
Copy link
Contributor Author

odersky commented Feb 8, 2018

test performance please

@dottybot
Copy link
Member

dottybot commented Feb 8, 2018

performance test scheduled: 1 job(s) in queue, 1 running.

@dottybot
Copy link
Member

dottybot commented Feb 8, 2018

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/3975/ to see the changes.

Benchmarks is based on merging with master (39980f3)

@odersky
Copy link
Contributor Author

odersky commented Feb 8, 2018

test performance please

@dottybot
Copy link
Member

dottybot commented Feb 8, 2018

performance test scheduled: 1 job(s) in queue, 1 running.

@dottybot
Copy link
Member

dottybot commented Feb 8, 2018

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/3975/ to see the changes.

Benchmarks is based on merging with master (39980f3)

1 similar comment
@dottybot
Copy link
Member

dottybot commented Feb 9, 2018

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/3975/ to see the changes.

Benchmarks is based on merging with master (39980f3)

When compiling dotc/typer/*.scala we observe a reduction of the number
of calls from ~3.7m to ~1.0m.
@odersky
Copy link
Contributor Author

odersky commented Feb 9, 2018

test performance please

@dottybot
Copy link
Member

dottybot commented Feb 9, 2018

performance test scheduled: 1 job(s) in queue, 0 running.

@odersky
Copy link
Contributor Author

odersky commented Feb 9, 2018

test performance please

@dottybot
Copy link
Member

dottybot commented Feb 9, 2018

performance test scheduled: 1 job(s) in queue, 1 running.

@odersky
Copy link
Contributor Author

odersky commented Feb 9, 2018

@liufengyun @allanrenucci Is there a timeout for performance tests? My second to last test contained a bug that makes it go basically forever. If there's no timeout we need to abort this so that other tests can run.

@liufengyun
Copy link
Contributor

@odersky Now it's killed, the other is running.

@odersky
Copy link
Contributor Author

odersky commented Feb 9, 2018

@liufengyun Thanks!

@dottybot
Copy link
Member

dottybot commented Feb 9, 2018

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/3975/ to see the changes.

Benchmarks is based on merging with master (5943331)

@odersky
Copy link
Contributor Author

odersky commented Feb 10, 2018

Subsumed by #3978

@odersky odersky closed this Feb 10, 2018
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.

3 participants