Skip to content

Reduce some allocations #5748

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

Merged
merged 19 commits into from
Jan 19, 2019
Merged

Reduce some allocations #5748

merged 19 commits into from
Jan 19, 2019

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jan 19, 2019

No description provided.

Add a miniphase, enabled by -Yinstrument-closures that counts
closure allocation by invoking Stats.doRecord.

Todo: Find a generally shippable way to use this. Right now it depends
on the compiler-internal utility class Stats.
Instead use NoAddr as a sentinel for None.
Avpid creation of moeratly hot closure
Without the optimization a comparison of value classes always boxing, since
the rhs operand is cast to Any. Most likely these are eliminated by escape
analysis, but by specializing here we make the job of the JOT compiler easier.
Again, these might be eliminated by escape analysis, but it does not
hurt to not allocate in the first place.
@odersky
Copy link
Contributor Author

odersky commented Jan 19, 2019

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/5748/ to see the changes.

Benchmarks is based on merging with master (e724058)

@odersky odersky requested a review from smarter January 19, 2019 10:34
@odersky
Copy link
Contributor Author

odersky commented Jan 19, 2019

Some smallish improvements. About 1-2% on compiler, other tests go up to 10%. So, this looks like a worthwhile thing to do.

@odersky
Copy link
Contributor Author

odersky commented Jan 19, 2019

test performance please

@dottybot
Copy link
Member

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

@dottybot
Copy link
Member

Performance test finished successfully:

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

Benchmarks is based on merging with master (9b74ac1)

envelope(source, span.startPos) // fill in children spans
() // Note: the `()` is there to prevent some inefficient code from being generated.
// Without it we get an allocation of a span here since the result type of the `if`
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this changes whether an allocation happens or not but it does make the code more efficient, I've opened an issue to track this that you could refer to in this comment: #5750

@odersky odersky merged commit 940a2c1 into scala:master Jan 19, 2019
@odersky odersky deleted the try-optimize-1 branch January 19, 2019 17:33
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.

4 participants