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

Simplify TypeComparer #9405

Merged
merged 5 commits into from
Aug 18, 2020
Merged

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jul 21, 2020

We create by accident a TypeComparer for every context. In my typer/*.scala benchmark, that created a million TypeComparers. Each of them has about 20 fields. We now create 3 TypeComparers overall for the same benchmark. This is done by having a stack of TypeComparers in use and re-using TypeComparers that were popped from the stack before.

@odersky odersky marked this pull request as ready for review July 21, 2020 20:19
@odersky
Copy link
Contributor Author

odersky commented Jul 21, 2020

test performance please

@dottybot
Copy link
Member

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

@dottybot
Copy link
Member

Performance test finished successfully:

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

Benchmarks is based on merging with master (239f4e3)

@odersky
Copy link
Contributor Author

odersky commented Jul 22, 2020

test performance please

@dottybot
Copy link
Member

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

@dottybot
Copy link
Member

Performance test finished successfully:

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

Benchmarks is based on merging with master (d6d374a)

@odersky
Copy link
Contributor Author

odersky commented Aug 18, 2020

test performance please

@odersky odersky assigned liufengyun and unassigned odersky Aug 18, 2020
@dottybot
Copy link
Member

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

@dottybot
Copy link
Member

Performance test finished successfully:

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

Benchmarks is based on merging with master (15ae0aa)

Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

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

LGTM


def reset(): Unit = {
for ((_, set) <- uniqueSets) set.clear()
errorTypeMsg.clear()
sources.clear()
sourceNamed.clear()
comparers.clear() // forces re-evaluation of top and bottom classes in TypeComparer
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to clear exploreContexts, and reset exploresInUse and comparersInUse to 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, every nested call with reset to the previous state after itself, even under exceptions. So there's nothing to do here.

@odersky odersky merged commit 693e81b into scala:master Aug 18, 2020
@odersky odersky deleted the optimize-typecomparer branch August 18, 2020 16:18
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