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

Avoid unnecessary file IO when computing positions #8935

Merged
merged 1 commit into from
Jun 9, 2020

Conversation

smarter
Copy link
Member

@smarter smarter commented May 10, 2020

The exists call introduced in #8883 and #8897 lead to a slowdown
noticeable in benchmarks. Replace them by a check on the content length,
since SourceFile caches its content this should avoid any unnecessary
IO operation. This required changing the way SourceFile handles empty
files to have it return an empty string instead of crashing.

@smarter
Copy link
Member Author

smarter commented May 10, 2020

test performance please

@smarter
Copy link
Member Author

smarter commented May 10, 2020

@liufengyun ^ is the benchmark bot down?

@dottybot
Copy link
Member

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

@liufengyun
Copy link
Contributor

@smarter I don't know why it missed the message, just update the message and it gets noticed.

@dottybot
Copy link
Member

Performance test finished successfully:

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

Benchmarks is based on merging with master (3937ddb)

@smarter
Copy link
Member Author

smarter commented May 10, 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/8935/ to see the changes.

Benchmarks is based on merging with master (3937ddb)

@smarter
Copy link
Member Author

smarter commented May 10, 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/8935/ to see the changes.

Benchmarks is based on merging with master (3937ddb)

@smarter
Copy link
Member Author

smarter commented May 11, 2020

test performance please

@dottybot
Copy link
Member

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

@liufengyun
Copy link
Contributor

The instability of the bot seems to be caused by the file system:

lampepfl/bench#170

@dottybot
Copy link
Member

Performance test finished successfully:

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

Benchmarks is based on merging with master (5fb865b)

The exists call introduced in scala#8883 and scala#8897 lead to a slowdown
noticeable in benchmarks. Replace them by a check on the content length,
since SourceFile caches its content this should avoid any unnecessary
IO operation. This required changing the way SourceFile handles empty
files to have it return an empty string instead of crashing.
@smarter
Copy link
Member Author

smarter commented Jun 1, 2020

test performance please

@dottybot
Copy link
Member

dottybot commented Jun 1, 2020

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

@dottybot
Copy link
Member

dottybot commented Jun 1, 2020

Performance test finished successfully:

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

Benchmarks is based on merging with master (c9a4196)

@smarter
Copy link
Member Author

smarter commented Jun 1, 2020

test performance please

@dottybot
Copy link
Member

dottybot commented Jun 1, 2020

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

@dottybot
Copy link
Member

dottybot commented Jun 2, 2020

Performance test finished successfully:

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

Benchmarks is based on merging with master (5404cc7)

@smarter smarter requested a review from nicolasstucki June 2, 2020 13:36
@smarter smarter assigned nicolasstucki and unassigned smarter Jun 2, 2020
@smarter
Copy link
Member Author

smarter commented Jun 8, 2020

Ping for review.

@nicolasstucki nicolasstucki merged commit 41388ee into scala:master Jun 9, 2020
@nicolasstucki nicolasstucki deleted the io-regression branch June 9, 2020 10:52
@liufengyun liufengyun mentioned this pull request Aug 14, 2020
7 tasks
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