Skip to content

Replace unused return values from Add() calls with add() and optimize loop traversal#170

Merged
deckarep merged 4 commits into
deckarep:mainfrom
mai7star:feature/use_add_instead_Add
Apr 25, 2026
Merged

Replace unused return values from Add() calls with add() and optimize loop traversal#170
deckarep merged 4 commits into
deckarep:mainfrom
mai7star:feature/use_add_instead_Add

Conversation

@mai7star

@mai7star mai7star commented Mar 17, 2026

Copy link
Copy Markdown
Contributor

Key Changes

This pull request makes two main performance optimizations:

1. Replace Add() with add() when return values are not used

When the boolean return value from Add() is not needed, the code now calls the private add() method instead. This eliminates unnecessary len() calls:

  • Example from NewThreadUnsafeSet():
    // Before: Uses public Add() method that checks length
    for _, item := range vals {
        s.Add(item)  // Returns bool, but value is discarded
    }
    
    // After: Uses private add() method without length check
    for i := range vs {
        s.add(vs[i])  // No unnecessary len() overhead
    }

2. Replace value-based slice iteration with index-based loops

Instead of using for _, val := range slice, the code now uses for i := range slice to access elements by index:

  • Example from Append():
    // Before: Range iteration with values
    for _, val := range v {
        (*s)[val] = struct{}{}
    }
    
    // After: Index-based iteration
    for i := range vs {
        s.add(vs[i])
    }

Files Modified

  1. bench_test.go - Added benchmarks for NewSet(), NewThreadUnsafeSet(), Append(), and other methods
  2. set.go - Optimized NewSet(), NewThreadUnsafeSet(), and related functions to use add() method
  3. threadunsafe.go - Updated Add(), Append(), and unmarshal methods to use index-based iteration and private add() calls

Additional Improvements

  • Parameter name changes for clarity: valsvs, item → (index-based), iis (for slices)
  • Simplified returns by eliminating intermediate variables
  • Changed receiver types from value to pointer for consistency in Add() and unmarshal methods

Benchmark

Before

BenchmarkNewSet-10                          	   87181	     13777 ns/op	   37032 B/op	       8 allocs/op
BenchmarkNewThreadUnsafeSet-10              	  149496	      8105 ns/op	   36944 B/op	       5 allocs/op
BenchmarkAddSafe-10                         	   30235	     38487 ns/op	   74496 B/op	      24 allocs/op
BenchmarkAddUnsafe-10                       	   33435	     36432 ns/op	   74464 B/op	      23 allocs/op
BenchmarkAppendSafe-10                      	   34560	     35259 ns/op	   74496 B/op	      24 allocs/op
BenchmarkAppendUnsafe-10                    	   33500	     36150 ns/op	   74464 B/op	      23 allocs/op

After

BenchmarkNewSet-10                          	  158842	      7448 ns/op	   36944 B/op	       5 allocs/op
BenchmarkNewThreadUnsafeSet-10              	  158649	      7544 ns/op	   36944 B/op	       5 allocs/op
BenchmarkAddSafe-10                         	   33783	     35403 ns/op	   74496 B/op	      24 allocs/op
BenchmarkAddUnsafe-10                       	   37732	     31670 ns/op	   74464 B/op	      23 allocs/op
BenchmarkAppendSafe-10                      	   38198	     31662 ns/op	   74496 B/op	      24 allocs/op
BenchmarkAppendUnsafe-10                    	   38355	     31581 ns/op	   74464 B/op	      23 allocs/op

Comment thread threadunsafe.go
prevLen := len(*s)
for _, val := range v {
(*s)[val] = struct{}{}
func (s *threadUnsafeSet[T]) Append(vs ...T) int {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks to me like an append() is needed as a counterpart to add(), since the optimized logic is used in multiple places:

func  (s *threadUnsafeSet[T]) Append(vs ...T) {
    prevLen := s.Cardinality()
	s.append(v)
	return prevLen != s.Cardinality()
}

func (s *threadUnsafeSet[T]) append(vs ...T) {
    for i := range vs {
        s.add(vs[i])
    }
}
// s.append can then be used in JSON/BSON unmarshaling

@mai7star mai7star force-pushed the feature/use_add_instead_Add branch from 2a24e45 to 9b7b974 Compare April 24, 2026 00:25
Comment thread set.go Outdated
@mai7star mai7star requested a review from ryclarke April 25, 2026 02:35

@ryclarke ryclarke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@deckarep deckarep merged commit 23f2c47 into deckarep:main Apr 25, 2026
15 checks passed
@mai7star mai7star deleted the feature/use_add_instead_Add branch April 25, 2026 04:58
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.

3 participants