Performance: Optimize sorted_set union/symmetric_difference and make bit extraction branchless

[Copilot]

Addresses 6 performance TODOs across sorted sets, bitstring operations, and array utilities.

Changes

SortedSet operations (sorted_set/set.mbt)

union() - Eliminate O(n) post-construction counting

• Previously built tree then iterated entire result to count size
• Now computes size incrementally during merge: (left_size + 1 + right_size)

symmetric_difference() - Remove intermediate allocations

• Previously: self.difference(other).union(other.difference(self)) (3 set allocations)
• Now: Direct iteration without intermediates (1 allocation)

Bit extraction (bytes/bitstring.mbt, array/bitstring.mbt)

Made 4 functions branchless for better CPU pipelining:

// Before: branch-based
if (bs.unsafe_get(byte_index) & bit_mask) != 0 { 1 } else { 0 }

// After: branchless bitwise
(bs.unsafe_get(byte_index).to_uint() >> bit_shift) & 1U

Array::split_at (builtin/array.mbt)

Removed unnecessary conditional - unsafe_blit with length 0 is safe:

// Before
if index != self.length() {
  UninitializedArray::unsafe_blit(v2.buffer(), 0, self.buffer(), index, len2)
}

// After
UninitializedArray::unsafe_blit(v2.buffer(), 0, self.buffer(), index, len2)

Split PR Structure

Changes split into 2 atomic commits for independent review:

• f5627ce: sorted_set + bitstring (3 files)
• c635380: array split_at (1 file)

Cherry-pick individually or merge together as needed.

Original prompt

Identify and suggest improvements to slow or inefficient code

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[peter-jerry-ye-code-review]

Potential integer overflow in signed bit extraction

Category
Correctness
Code Snippet
// Extract bit and convert to signed: 0 -> 0, 1 -> -1
(((byte_val >> bit_shift) & 1) * -1) | 0
Recommendation
Use conditional negation or proper sign extension: if ((byte_val >> bit_shift) & 1) != 0 { -1 } else { 0 } or -(((byte_val >> bit_shift) & 1).to_int())
Reasoning
The expression (((byte_val >> bit_shift) & 1) * -1) | 0 may not behave correctly for all integer types and could be confusing. The | 0 operation is unnecessary since we're already working with integers, and the multiplication approach is less clear than direct conditional logic.

Symmetric difference still has O(n²) complexity due to repeated contains() calls

Category
Performance
Code Snippet
let ret = new()
self.each(x => if !other.contains(x) { ret.add(x) })
other.each(x => if !self.contains(x) { ret.add(x) })
Recommendation
Consider implementing a tree-based symmetric difference algorithm similar to the union implementation, using split and join operations to maintain O(n log n) complexity
Reasoning
While this eliminates intermediate allocations, it introduces O(n²) complexity in worst case since each contains() call is O(log n) and we do this for each element. The original approach using difference() + union() likely had better algorithmic complexity.

Missing helper function documentation and unclear bit manipulation logic

Category
Maintainability
Code Snippet
fn[V] count_nodes(node : Node[V]?) -> Int {
match node {
None => 0
Some({ left, right, .. }) => 1 + count_nodes(left) + count_nodes(right)
}
}
Recommendation
Add documentation comment explaining the purpose: ///| Count the total number of nodes in a tree rooted at the given node
Reasoning
The new helper function lacks documentation, and the bit manipulation changes remove helpful comments. Good documentation is especially important for performance-critical code that might be less intuitive.

[coveralls]

Pull Request Test Coverage Report for Build 1943

Details

• 21 of 21 (100.0%) changed or added relevant lines in 4 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.004%) to 91.306%

---

Totals
Change from base Build 1938:	-0.004%
Covered Lines:	10323
Relevant Lines:	11306

---

💛 - Coveralls

[Copilot]

Pull Request Overview

This PR addresses 6 performance TODOs with optimizations to sorted set operations, bitstring bit extraction, and array utilities. The main changes include computing set union size incrementally, replacing symmetric_difference's intermediate allocations with direct iteration, making bit extraction branchless, and removing a conditional from array split_at.

• Optimized union() to compute size during tree construction instead of post-construction traversal
• Replaced symmetric_difference() implementation to avoid creating two intermediate sets
• Made 4 bitstring extraction functions branchless for better CPU pipelining

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
sorted_set/set.mbt	Modified union() to track size during merge; rewrote symmetric_difference() to avoid intermediate allocations; added count_nodes() helper
bytes/bitstring.mbt	Converted unsafe_extract_bit and unsafe_extract_bit_signed to branchless implementations
array/bitstring.mbt	Converted unsafe_extract_bit and unsafe_extract_bit_signed to branchless implementations
builtin/array.mbt	Removed unnecessary conditional from split_at when blitting empty arrays

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The count_nodes() calls defeat the optimization's purpose. When one subtree is exhausted during union, count_nodes() still traverses the entire remaining tree in O(n) time, which is what the original TODO aimed to eliminate. Consider storing size information in each node, or accumulating sizes from split() operations to avoid this traversal.

[Copilot]

The new implementation may be slower than the original for large sets. Each add() call maintains tree balance (O(log n)), so iterating through all elements has O(n log n + m log m) complexity for tree construction, plus O(n log m + m log n) for the contains() checks. The original approach using union() on pre-built difference sets leverages more efficient tree-merge operations. Consider benchmarking both approaches or using a bulk-build strategy.

Suggested change

	let ret = new()
	self.each(x => if !other.contains(x) { ret.add(x) })
	other.each(x => if !self.contains(x) { ret.add(x) })
	ret
	self.difference(other).union(other.difference(self))

[Yu-zh]

The bit extraction part looks good to me.