Skip to content

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)

if (!mySpan.exists && span.exists) {
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