Skip to content

Optimize RNG, visited tracking, and arrow representation - #1

Open
Nanosplitter wants to merge 2 commits into
masterfrom
claude/code-optimization-review-3wbgz8
Open

Optimize RNG, visited tracking, and arrow representation#1
Nanosplitter wants to merge 2 commits into
masterfrom
claude/code-optimization-review-3wbgz8

Conversation

@Nanosplitter

Copy link
Copy Markdown
Owner

Summary

This PR refactors core performance-critical components of the simulation engine, replacing slow standard library functions with optimized implementations and improving data structure efficiency.

Key Changes

  • Replace rand() with xorshift32 PRNG: Implemented a fast inline xorshift32 pseudo-random number generator with unbiased range selection via multiply-shift, eliminating the bias and overhead of rand() % b

  • Optimize visited state tracking: Replaced boolean arrays with a stamp-based approach using uint16_t counters. Instead of clearing the entire visited table each run, a monotonically increasing stamp marks visited states. This avoids O(HEIGHT×WIDTH×NUMDIR) memset operations on every simulation run, with automatic wraparound handling

  • Introduce Arrow struct: Replaced vector<int>{y, x, dir} with a trivially copyable POD struct. This provides:

    • Type-safe field access (.y, .x, .dir instead of .at(0), .at(1), .at(2))
    • Contiguous memory layout for vectors
    • Allocation-free copying
    • Bounds-check-free indexing
  • Simplify arrow generation: Refactored genRandArrows() to use partial Fisher-Yates shuffling (O(numRandArrows) instead of O(validCells.size())), eliminating unnecessary full-array shuffles

  • Remove unused code: Deleted getArrString() from Simulation class, unused copyUsedCells() method, and numerous commented-out debug statements

  • Improve code clarity: Changed generateNewArrows() to modify in-place (void return) rather than returning a copy, simplified array initialization with memset, and replaced verbose loops with cleaner implementations

Implementation Details

  • The stamp-based visited tracking wraps around at 65535 runs, triggering a single reset to avoid overflow
  • Arrow struct enables direct array indexing without bounds checks while maintaining type safety
  • Partial Fisher-Yates reduces random number generation calls and array operations during initialization

https://claude.ai/code/session_01LRo9aeU3LgH3nrfwxtZqMA

Speed up the fixed-time-budget search so more simulations run per turn:

- getScore: replace the per-call clear of the visited table with a
  generation "stamp" (uint16_t) that goes stale by incrementing, so the
  760-entry table never has to be zeroed on the hot path.
- Represent arrows as a trivially-copyable Arrow POD instead of
  vector<vector<int>>, removing per-arrow heap allocations and the
  bounds-checked .at() accesses throughout.
- play(): take arrows by const&, drop the pointless per-call reverse, and
  initialize numSims.
- addArrow(): update usedCells incrementally instead of rebuilding the
  whole table from the arrow list each call.
- Replace rand()%b (slow, biased) with an inline xorshift + multiply-shift
  rnd(); seed via seedRng().
- generateNewArrows(): take the state vector by reference instead of
  copying it every generation.
- genRandArrows(): partial Fisher-Yates over the used prefix instead of a
  full random_shuffle (also fixes C++17 removal of random_shuffle).
- copyStates(): copy the best state into the others (previous version
  copied in the wrong direction, corrupting the best state).
- Remove unused Robot::cells and Robot::visitedReset arrays.
@Nanosplitter
Nanosplitter force-pushed the claude/code-optimization-review-3wbgz8 branch from a6a4a09 to e1e25fc Compare July 22, 2026 16:18
Robot::getScore was undercounting robots that terminate by re-entering a
previously-seen (position, direction) state, versus what the real game
scores. Two bugs compounded:

- On a repeat, the function returned `score` instead of `score + 1`. Per
  the game's turn order, score is incremented before the move/arrow/death
  check each turn, so a robot is credited for the turn it dies on even
  when it dies by hitting void or a repeat - the void case already did
  `score + 1`, the repeat case did not.
- The robot's own starting state (position + direction, after any
  arrow-under-start adjustment) was never marked as visited before the
  simulation loop began, so a path that looped exactly back to its start
  wasn't recognized as a repeat there, letting it go one extra step and
  pick up a bogus extra point before eventually dying elsewhere.

Verified against the real game: applying the reported "Best Arrows" for a
4-robot board where all four robots die by repeat, the old code scored
572 while the true score was 575. All four robots hit the missing +1;
one of them also loops back through its own start state, so the two bugs
net-cancel for that one robot. With both fixes the simulator reproduces
575 exactly. Also re-verified the fixed, stamp-based getScore against an
independent plain-array reference implementing the corrected rules across
200,000 random board/arrow configurations with zero mismatches.
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.

1 participant