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

Replace IntervalTree with Vec #151

Merged
merged 3 commits into from
Dec 4, 2019
Merged

Conversation

philipc
Copy link
Contributor

@philipc philipc commented Dec 4, 2019

IntervalTree doesn't perform as well as a simple tree of Vecs. Note that a Vec tree can't handle partially overlapping intervals, but we don't need that.

 name                                        before ns/iter  after ns/iter  diff ns/iter   diff %  speedup
 context_new_and_query_location_rc           2,427,320       2,432,182             4,862    0.20%   x 1.00
 context_new_and_query_location_slice        765,873         758,006              -7,867   -1.03%   x 1.01
 context_new_and_query_with_functions_rc     2,993,102       2,959,278           -33,824   -1.13%   x 1.01
 context_new_and_query_with_functions_slice  1,133,112       1,152,324            19,212    1.70%   x 0.98
 context_new_parse_functions_rc              31,235,025      33,387,535        2,152,510    6.89%   x 0.94
 context_new_parse_functions_slice           23,754,161      26,858,657        3,104,496   13.07%   x 0.88
 context_new_parse_lines_rc                  11,725,686      11,711,061          -14,625   -0.12%   x 1.00
 context_new_parse_lines_slice               7,337,852       7,283,802           -54,050   -0.74%   x 1.01
 context_new_rc                              2,207,283       2,162,930           -44,353   -2.01%   x 1.02
 context_new_slice                           613,189         603,892              -9,297   -1.52%   x 1.02
 context_query_location_rc                   728,349         733,083               4,734    0.65%   x 0.99
 context_query_location_slice                721,907         729,294               7,387    1.02%   x 0.99
 context_query_with_functions_rc             4,292,707       1,952,886        -2,339,821  -54.51%   x 2.20
 context_query_with_functions_slice          4,302,388       1,744,765        -2,557,623  -59.45%   x 2.47
 new                                         28,964          27,013               -1,951   -6.74%   x 1.07
 new_unresolved_and_resolve_separate         29,332          26,986               -2,346   -8.00%   x 1.09
 trace_and_resolve_callback                  13,379          11,471               -1,908  -14.26%   x 1.17
 trace_and_resolve_separate                  11,013          9,801                -1,212  -11.01%   x 1.12

For the test parse_functions_slice, memory usage increased from 17.4MB to 19.7MB.

Also save some memory by using Box for line rows.

Useful for profiling the parsing code.
 name                                        before ns/iter  after ns/iter  diff ns/iter   diff %  speedup
 context_new_and_query_location_rc           2,427,320       2,432,182             4,862    0.20%   x 1.00
 context_new_and_query_location_slice        765,873         758,006              -7,867   -1.03%   x 1.01
 context_new_and_query_with_functions_rc     2,993,102       2,959,278           -33,824   -1.13%   x 1.01
 context_new_and_query_with_functions_slice  1,133,112       1,152,324            19,212    1.70%   x 0.98
 context_new_parse_functions_rc              31,235,025      33,387,535        2,152,510    6.89%   x 0.94
 context_new_parse_functions_slice           23,754,161      26,858,657        3,104,496   13.07%   x 0.88
 context_new_parse_lines_rc                  11,725,686      11,711,061          -14,625   -0.12%   x 1.00
 context_new_parse_lines_slice               7,337,852       7,283,802           -54,050   -0.74%   x 1.01
 context_new_rc                              2,207,283       2,162,930           -44,353   -2.01%   x 1.02
 context_new_slice                           613,189         603,892              -9,297   -1.52%   x 1.02
 context_query_location_rc                   728,349         733,083               4,734    0.65%   x 0.99
 context_query_location_slice                721,907         729,294               7,387    1.02%   x 0.99
 context_query_with_functions_rc             4,292,707       1,952,886        -2,339,821  -54.51%   x 2.20
 context_query_with_functions_slice          4,302,388       1,744,765        -2,557,623  -59.45%   x 2.47
 new                                         28,964          27,013               -1,951   -6.74%   x 1.07
 new_unresolved_and_resolve_separate         29,332          26,986               -2,346   -8.00%   x 1.09
 trace_and_resolve_callback                  13,379          11,471               -1,908  -14.26%   x 1.17
 trace_and_resolve_separate                  11,013          9,801                -1,212  -11.01%   x 1.12

For the test `parse_functions_slice`, memory usage increased from 17.4MB to 19.7MB.
@philipc philipc requested a review from fitzgen December 4, 2019 09:06
@coveralls
Copy link

Coverage Status

Coverage increased (+6.4%) to 76.95% when pulling 8c79143 on philipc:no-interval-tree into 93163d2 on gimli-rs:master.

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

Very nice!

@fitzgen fitzgen merged commit 2674396 into gimli-rs:master Dec 4, 2019
@philipc philipc deleted the no-interval-tree branch December 5, 2019 04:42
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.

None yet

3 participants