Skip to content

fix: replace global math/rand with per-filter RNG for thread safety#30

Merged
shaia merged 1 commit into
mainfrom
fix/thread-safety-math-rand
Nov 24, 2025
Merged

fix: replace global math/rand with per-filter RNG for thread safety#30
shaia merged 1 commit into
mainfrom
fix/thread-safety-math-rand

Conversation

@shaia
Copy link
Copy Markdown
Owner

@shaia shaia commented Nov 24, 2025

Replace the global math/rand package with math/rand/v2 and use a per-filter RNG instance to eliminate mutex contention under concurrent access.

Changes to filter.go:

  • Upgrade from math/rand to math/rand/v2
  • Add rng field to simdFilter struct
  • Initialize RNG with PCG algorithm in New()
  • Update relocate() to use f.rng.IntN() instead of rand.Intn()

Changes to filter_test.go:

  • TestFilterRNGInitialization: Verify each filter has its own RNG
  • TestFilterConcurrentMultipleFilters: Test concurrent ops on multiple filters
  • TestFilterConcurrentInsertWithRelocation: Stress test RNG in relocate()
  • TestFilterRNGIndependence: Verify filters produce independent sequences
  • TestFilterConcurrentMixedOperations: Test mixed insert/lookup/delete concurrency

The previous implementation used the global rand.Intn() function which holds a global mutex, causing contention when multiple goroutines access different filter instances concurrently. Each filter now maintains its own RNG instance, eliminating this bottleneck while maintaining proper randomization for cuckoo hashing relocations.

All tests pass including race detector.

Replace the global math/rand package with math/rand/v2 and use a
per-filter RNG instance to eliminate mutex contention under concurrent
access.

Changes to filter.go:
- Upgrade from math/rand to math/rand/v2
- Add rng field to simdFilter struct
- Initialize RNG with PCG algorithm in New()
- Update relocate() to use f.rng.IntN() instead of rand.Intn()

Changes to filter_test.go:
- TestFilterRNGInitialization: Verify each filter has its own RNG
- TestFilterConcurrentMultipleFilters: Test concurrent ops on multiple filters
- TestFilterConcurrentInsertWithRelocation: Stress test RNG in relocate()
- TestFilterRNGIndependence: Verify filters produce independent sequences
- TestFilterConcurrentMixedOperations: Test mixed insert/lookup/delete concurrency

The previous implementation used the global rand.Intn() function which
holds a global mutex, causing contention when multiple goroutines access
different filter instances concurrently. Each filter now maintains its
own RNG instance, eliminating this bottleneck while maintaining proper
randomization for cuckoo hashing relocations.

All tests pass including race detector.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves thread safety and concurrent performance by replacing the global math/rand package with math/rand/v2 and introducing per-filter RNG instances. This eliminates mutex contention that occurred when multiple goroutines accessed different filter instances concurrently.

Key Changes:

  • Upgraded from math/rand to math/rand/v2 with PCG algorithm for better randomness
  • Added rng field to simdFilter struct with per-instance initialization
  • Added five comprehensive concurrent test cases verifying RNG independence and thread safety

Reviewed changes

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

File Description
internal/filter/filter.go Added per-filter RNG field, initialized with PCG algorithm in New(), and updated relocate() to use instance RNG instead of global rand
internal/filter/filter_test.go Added comprehensive concurrent tests covering RNG initialization, multi-filter operations, relocation stress testing, RNG independence, and mixed concurrent operations

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

@shaia shaia merged commit f89428f into main Nov 24, 2025
21 checks passed
@shaia shaia deleted the fix/thread-safety-math-rand branch November 24, 2025 13:18
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