Skip to content

Improve deleteat!(x,::Integer) - #117

Merged
quinnj merged 3 commits into
JuliaData:mainfrom
laborg:improvedeleteat
Oct 3, 2026
Merged

quinnj merged 3 commits into
JuliaData:mainfrom
laborg:improvedeleteat

Conversation

@laborg

@laborg laborg commented Dec 25, 2024

Copy link
Copy Markdown
Contributor

We only need to remove one element from a ChainedVector and can therefore special case the fact that if this element is a single entry in a chunk the chunk can be removed completely. This reliefs us from the slow cleanup!() call.
As a result dependent functions performance improve:

pop!() with many chunks

julia> function popall(x)
           while length(x) >0
               pop!(x)
           end
       end
popall (generic function with 1 method)

julia> popall(ChainedVector([[1]])); # compilation

julia> cv = ChainedVector([rand(Int,5) for _ in 1:20000]);

julia> @time popall(cv) # main
  1.043174 seconds

julia> @time popall(cv) # pr
  0.003701 seconds

popfirst!() with many chunks

julia> function popall(x)
           while length(x) >0
               popfirst!(x)
           end
       end
popall (generic function with 1 method)

julia> popall(ChainedVector([[1]])); # compilation

julia> cv = ChainedVector([rand(Int,5) for _ in 1:20000]);

julia> @time popall(cv) # main
  1.423213 seconds

julia> @time popall(cv) # pr
  0.128358 seconds 

popfirst!() with single chunk

julia> function popall(x)
           while length(x) >0
               popfirst!(x)
           end
       end
popall (generic function with 1 method)

julia> popall(ChainedVector([[1]])); # compilation

julia> cv = ChainedVector([rand(Int,100000)]);

julia> @time popall(cv)
  0.002024 seconds # main

julia> @time popall(cv)
  0.001135 seconds # pr

We only need to remove one element from a ChainedVector and can therefore special case the fact that if this element is a single entry in a chunk the chunk can be removed completely. This reliefs us from the slow `cleanup!()` call.

@quinnj quinnj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[Reviewed by AI] Reviewed f229bc0caaeda22bfbea9a5f9605b01c69e14cbc. No actionable findings. The full suite and 10,024 focused deletion checks against Base passed locally on Julia 1.13/macOS arm64, including empty input chunks, singleton boundaries, custom indices, pop!, and popfirst!.

Compatibility note: dropping a singleton chunk leaves an external reference to that backing vector unchanged; the logical ChainedVector result remains correct.

quinnj and others added 2 commits September 29, 2026 10:27
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.65%. Comparing base (fa840f9) to head (01b29a6).
⚠️ Report is 19 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #117      +/-   ##
==========================================
+ Coverage   94.82%   95.65%   +0.82%     
==========================================
  Files           5        5              
  Lines        1043     1058      +15     
==========================================
+ Hits          989     1012      +23     
+ Misses         54       46       -8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@quinnj quinnj mentioned this pull request Sep 29, 2026

@quinnj quinnj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed 01b29a6db7d645902896b56cb6187bfde67b97ad and its clean local integration with current main 7752d36a6457e82d649032bf5895cc54fa72bd8a. No actionable findings in the change. The complete suite passes on Julia 1.12.6/macOS arm64, and 18,318 deterministic deletion checks agree with Vector for integer, unsigned, and ChainedVectorIndex positions, empty input chunks, and singleton boundaries.

Five warmed samples confirm the performance gain for draining 20,000 five-element chunks: pop! median 0.8091 s to 0.003950 s (205x); popfirst! 1.050 s to 0.1339 s (7.84x). Single-chunk and singleton-chunk cases also improve. Both revisions allocate 448 bytes per measured drain.

Compatibility note: removing a singleton detaches its backing vector without emptying it, so an external reference keeps its value. The logical ChainedVector result is correct.

The extra checks also found an existing empty resize failure on unchanged main; I have isolated it for a separate fix.

AI disclosure: This work was prepared with assistance from OpenAI Codex.

@quinnj
quinnj merged commit c1e31bb into JuliaData:main Oct 3, 2026
10 checks passed
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.

2 participants