Skip to content

Commit

Permalink
* Optimize SourceBuffer line and column handling (memory and perfor…
Browse files Browse the repository at this point in the history
…mance)

`#line_begins`:
- store `line_begin` instead of `[line_begin, index]`
- simplify code by storing an additional "virtual" line_begin at the end

caches for `position` => line / column: Use same cache instead of two.

`#decompose_position`: use cache there too

I added tests for behavior of some methods with out-of-range arguments.
Behavior arguably less bad.
  • Loading branch information
marcandre committed Oct 28, 2020
1 parent 9aeabf9 commit 0202bc9
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 29 deletions.
61 changes: 34 additions & 27 deletions lib/parser/source/buffer.rb
Expand Up @@ -114,8 +114,7 @@ def initialize(name, first_line = 1, source: nil)
@slice_source = nil

# Cache for fast lookup
@line_for_position = {}
@column_for_position = {}
@line_index_for_position = {}

self.source = source if source
end
Expand Down Expand Up @@ -207,9 +206,10 @@ def slice(range)
# @return [[Integer, Integer]] `[line, column]`
#
def decompose_position(position)
line_no, line_begin = line_for(position)
line_index = line_index_for_position(position)
line_begin = line_begins[line_index]

[ @first_line + line_no, position - line_begin ]
[ @first_line + line_index , position - line_begin ]
end

##
Expand All @@ -220,10 +220,7 @@ def decompose_position(position)
# @api private
#
def line_for_position(position)
@line_for_position[position] ||= begin
line_no, _ = line_for(position)
@first_line + line_no
end
line_index_for_position(position) + @first_line
end

##
Expand All @@ -234,10 +231,8 @@ def line_for_position(position)
# @api private
#
def column_for_position(position)
@column_for_position[position] ||= begin
_, line_begin = line_for(position)
position - line_begin
end
line_index = line_index_for_position(position)
position - line_begins[line_index]
end

##
Expand Down Expand Up @@ -278,15 +273,13 @@ def source_line(lineno)
# @raise [IndexError] if `lineno` is out of bounds
#
def line_range(lineno)
index = lineno - @first_line + 1
if index <= 0 || index > line_begins.size
index = lineno - @first_line
if index < 0 || index + 1 >= line_begins.size
raise IndexError, 'Parser::Source::Buffer: range for line ' \
"#{lineno} requested, valid line numbers are #{@first_line}.." \
"#{@first_line + line_begins.size - 1}"
elsif index == line_begins.size
Range.new(self, line_begins[-index][1], @source.size)
"#{@first_line + line_begins.size - 2}"
else
Range.new(self, line_begins[-index][1], line_begins[-index - 1][1] - 1)
Range.new(self, line_begins[index], line_begins[index + 1] - 1)
end
end

Expand All @@ -303,27 +296,41 @@ def source_range
# @return [Integer]
#
def last_line
line_begins.size + @first_line - 1
line_begins.size + @first_line - 2
end

private

# @returns [0, line_begin_of_line_1, ..., source.size + 1]
def line_begins
unless @line_begins
@line_begins, index = [ [ 0, 0 ] ], 0

@line_begins ||= begin
@line_begins = [0]
index = 0
while index = @source.index("\n".freeze, index)
index += 1
@line_begins.unshift [ @line_begins.length, index ]
@line_begins << index
end
@line_begins << @source.size + 1
end
end

@line_begins
# @returns 0-based line index of position
def line_index_for_position(position)
@line_index_for_position[position] ||= bsearch(line_begins, position) - 1
end

def line_for(position)
line_begins.bsearch do |line, line_begin|
line_begin <= position
if Array.method_defined?(:bsearch_index) # RUBY_VERSION >= 2.3
def bsearch(line_begins, position)
line_begins.bsearch_index do |line_begin|
position < line_begin
end || line_begins.size - 1 # || only for out of bound values
end
else
def bsearch(line_begins, position)
@line_range ||= 0...line_begins.size
@line_range.bsearch do |i|
position < line_begins[i]
end || line_begins.size - 1 # || only for out of bound values
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions test/test_source_buffer.rb
Expand Up @@ -76,8 +76,8 @@ def test_decompose_position
assert_equal [1, 1], @buffer.decompose_position(1)
assert_equal [2, 0], @buffer.decompose_position(2)
assert_equal [3, 1], @buffer.decompose_position(7)
assert_equal [3, 71], @buffer.decompose_position(42)
assert_raises { @buffer.decompose_position(-42) }
assert_equal [3, 36], @buffer.decompose_position(42)
assert_equal [0, -52], @buffer.decompose_position(-42)
end

def test_decompose_position_mapped
Expand Down

0 comments on commit 0202bc9

Please sign in to comment.