Skip to content

Add -Ydebug-tree-with-id n #6363

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 1 commit into from
Apr 26, 2019

Conversation

nicolasstucki
Copy link
Contributor

This postion allow descovering the place creation of a tree given its unique id.
Adding is as a setting to simplify debuging on a released version of dotty.

@nicolasstucki nicolasstucki self-assigned this Apr 23, 2019
@nicolasstucki
Copy link
Contributor Author

test performance please

@dottybot
Copy link
Member

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

This postion allow descovering the place creation of a tree given its unique id.
Adding is as a setting to simplify debuging on a released version of dotty.
@nicolasstucki nicolasstucki force-pushed the add-tree-id-debug-setting branch from 5c34915 to 5bb1ee3 Compare April 23, 2019 15:09
@dottybot
Copy link
Member

Performance test finished successfully:

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

Benchmarks is based on merging with master (2a033aa)

val stack = Thread.currentThread().getStackTrace().map("> " + _)
System.err.println(stack.mkString(s"> Debug tree (id=${Positioned.debugId}) creation \n> $this\n", "\n", "\n"))
}
printTrace()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the logic abstracted into a method instead of inlining it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

uniqueId_= is probably a hot path as each tree creation will have a call to it. I put the code contained in this debug branch to make the bytecode of uniqueId_= smaller and probably simple to optimize from the JVM perspective.

@nicolasstucki
Copy link
Contributor Author

test performance please

@dottybot
Copy link
Member

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

@nicolasstucki nicolasstucki marked this pull request as ready for review April 24, 2019 13:22
@nicolasstucki nicolasstucki requested a review from odersky April 24, 2019 13:22
@dottybot
Copy link
Member

Performance test finished successfully:

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

Benchmarks is based on merging with master (2b95f1d)

@odersky odersky merged commit 1521576 into scala:master Apr 26, 2019
@ghost ghost removed the stat:needs review label Apr 26, 2019
@allanrenucci allanrenucci deleted the add-tree-id-debug-setting branch April 26, 2019 16:01
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