Closed CathalMullan closed 1 month ago
Attention: Patch coverage is 94.92386%
with 10 lines
in your changes missing coverage. Please review.
:white_check_mark: All tests successful. No failed tests found.
Files with missing lines | Patch % | Lines |
---|---|---|
src/node.rs | 92.72% | 4 Missing :warning: |
src/node/delete.rs | 85.00% | 3 Missing :warning: |
src/node/insert.rs | 96.42% | 0 Missing and 2 partials :warning: |
src/node/optimize.rs | 97.82% | 1 Missing :warning: |
Files with missing lines | Coverage Δ | |
---|---|---|
src/node/display.rs | 88.00% <100.00%> (ø) |
|
src/node/search.rs | 79.20% <100.00%> (ø) |
|
src/router.rs | 85.63% <100.00%> (+0.24%) |
:arrow_up: |
src/node/optimize.rs | 97.82% <97.82%> (ø) |
|
src/node/insert.rs | 94.22% <96.42%> (+0.42%) |
:arrow_up: |
src/node/delete.rs | 79.84% <85.00%> (+0.34%) |
:arrow_up: |
src/node.rs | 92.72% <92.72%> (ø) |
Comparing 165-optimize-insert-speed
(4cd55d2) with main
(b2d615c)
⚡ 2
improvements
✅ 17
untouched benchmarks
Benchmark | main |
165-optimize-insert-speed |
Change | |
---|---|---|---|---|
⚡ | gitlab delete benchmarks/wayfind |
6,891.9 ms | 21 ms | ×330 |
⚡ | gitlab insert benchmarks/wayfind |
849.9 ms | 16 ms | ×53 |
Tried to reduce how often we sort by tracking changes. Also updated the quicks and performed sort in a single walk of the tree.
Removing the 'sort' logic results in -12% to the time. Removing the 'quicks' logic results in -3% to the time.
So the real issue here is walking the tree, not the cost of any one action. The walk itself takes up 85% of the time.
We should aim to be able to say 'this whole branch is already optimized'.
If we end up creating a 'Children' struct to wrap the vec, maybe we should consider more refactors elsewhere.
Ideally we'd have some way to know if a child and it's children have already been optimized, so we can short-circuit the functions.
Saying that - I'd rather be overly cautious if unsure on the approach.