-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Conversation
test performance please |
@liufengyun ^ is the benchmark bot down? |
performance test scheduled: 1 job(s) in queue, 1 running. |
@smarter I don't know why it missed the message, just update the message and it gets noticed. |
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/8935/ to see the changes. Benchmarks is based on merging with master (3937ddb) |
test performance please |
performance test scheduled: 1 job(s) in queue, 0 running. |
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/8935/ to see the changes. Benchmarks is based on merging with master (3937ddb) |
test performance please |
performance test scheduled: 1 job(s) in queue, 0 running. |
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/8935/ to see the changes. Benchmarks is based on merging with master (3937ddb) |
test performance please |
performance test scheduled: 1 job(s) in queue, 1 running. |
The instability of the bot seems to be caused by the file system: |
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.
test performance please |
performance test scheduled: 2 job(s) in queue, 1 running. |
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/8935/ to see the changes. Benchmarks is based on merging with master (c9a4196) |
test performance please |
performance test scheduled: 3 job(s) in queue, 1 running. |
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/8935/ to see the changes. Benchmarks is based on merging with master (5404cc7) |
Ping for review. |
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.