Omit unnecessary field from location range #1788
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR reduces memory usage for
RBS::Location
by omitting unnecessary fields.Previously
RBS::Location
had byte pos, char pos, line, and column. But they are caches except for char pos. These caches are rarely accessed. So we can drop them fromRBS::Location
.design doc: https://hackmd.io/QNkMU6roTqSn_iXb2ENhsA
Benchmarking
I used
benchmark/memory_new_rails_env.rb
for the benchmark.It decreases 15% memory.
This PR also fixes two problems of the current implementations.
First,
Buffer#pos_to_loc
returns the wrong line and column for the EOS position of a string without the end of a newline. For example, the location forabc|
(|
is the cursor) should be[1, 3]
, but#pos_to_loc
unexpectedly returns[2, 0]
.This problem is fixed in 81b44e1
Second,
#pos_to_loc
returns an unexpected value when the position is located EOF.The cause is that the parser determines the length of EOF as 1. Then, the
RBS::Location
of thepEOF
token contains an out-of-range position. Because an EOF has a length in C String, but it doesn't have a length in Ruby String.I changed the length of EOF token to zero to fix this problem.
16006c2