Skip to content

Optimize copy method from O(N log N) to O(N)#38

Merged
jordanmontt merged 2 commits intopharo-containers:mainfrom
HossamSaberr:main
Mar 3, 2026
Merged

Optimize copy method from O(N log N) to O(N)#38
jordanmontt merged 2 commits intopharo-containers:mainfrom
HossamSaberr:main

Conversation

@HossamSaberr
Copy link
Contributor

Fixes #37

This PR optimizes the copy functionality of CTAVLTree by replacing the O(N log N) element-by-element re-insertion with a O(N) structural deep copy.

Changes:

  1. Removed the custom copy method in CTAVLTree that relied on inOrderDo:, allowing the system to use the standard shallowCopy -> postCopy.
  2. Implemented postCopy in CTAVLTree to initiate the root duplication.
  3. Implemented a recursive copy in CTAVLNode that directly duplicates left and right children while manually injecting the height instance variable, completely bypassing add: balancing math and rotations.
  4. Added an instant copy return for CTAVLNilNode.
  5. Added test cases (testCopyEmptyTree, testCopySingleNode, testCopyStructureAndIsolation, testCopyLargeTree) to prove structural integrity, exact shape matching, and memory isolation.

Performance Proof:
Benchmarking the old implementation vs. the new O(N) implementation shows constant factor reduction and linear scaling:

N (Elements) Old copy O(N log N) New copy O(N) Improvement
10,000 9 ms 4 ms ~2.2x faster
20,000 22 ms 9 ms ~2.4x faster
40,000 40 ms 33 ms ~1.2x faster
80,000 190 ms 63 ms ~3.0x faster
160,000 339 ms 224 ms ~1.5x faster

Benchmark Script & Visual Proof

You can reproduce these results using the following Playground script:

| sizes results |
sizes := #(10000 20000 40000 80000 160000).
results := OrderedCollection new.

sizes do: [ :n |
    | tree data copyTime |
    tree := CTAVLTree new.
    data := (1 to: n) asArray shuffled.
    tree addAll: data.
    
    copyTime := Time millisecondsToRun: [ tree copy ].
    
    results add: ('N=', n asString, ' took ', copyTime asString, ' ms').
].

results inspect.

Before (O(N log N)):
image

After (O(N)):
image

@HossamSaberr
Copy link
Contributor Author

Done, I've added testCopyLeafNode to ensure the individual nodes copy correctly. I also took the opportunity to fix the CI failure by refactoring CTAVLNode >> copy into postCopy, which is much cleaner anyway. The CI should be Green now

@jordanmontt jordanmontt merged commit 079895e into pharo-containers:main Mar 3, 2026
12 of 15 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.

Optimize copy method from O(n log n) to O(n)

2 participants