Minor optimizations

[oschwald]

• Increase size of string cache
• Optimize map[string]string decoding and refactor string value decoding

Summary by CodeRabbit

• Refactor

  • Increased string cache capacity (larger cache table) to improve performance under load
  • Optimized decoding for string-to-string maps with a fast path to reduce allocations

• Tests

  • Added comprehensive tests for string-to-string map decoding, including nested cases and edge conditions

[coderabbitai]

Walkthrough

Introduces a fast path for decoding map[string]string, adds DataDecoder.decodeStringValue to centralize string/pointer decoding, and increases the internal string cache size from 512 to 4096 entries. No public API signatures were changed.

Changes

Cohort / File(s)	Summary
String cache internal/decoder/string_cache.go, CHANGELOG.md	Expanded the fixed-size string cache from 512 to 4096 entries (no behavioral changes besides larger table).
String decoding core internal/decoder/data_decoder.go, internal/decoder/decoder.go	Added DataDecoder.decodeStringValue(offset) (string, uint, error) to handle control-data, single-pointer following, type verification, and string decoding; ReadString() updated to call it.
Map[string]string fast path & unmarshaler caching internal/decoder/reflection.go	Added mapStringStringType, unmarshalerType, unmarshalerTypeCache, mayImplementUnmarshaler() and ReflectionDecoder.decodeMapStringString(); unmarshalMap dispatches to the specialized fast path for map[string]string and uses cached checks for Unmarshaler support.
Tests internal/decoder/reflection_test.go	Added TestMapStringString and TestMapStringStringNested covering decoding of map[string]string (various cases) and nested-map scenarios, including error cases.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 I nibble bytes and chase the trail of pointers light,
Keys and values bound in hop and sprightly sight,
The cache grows wider, strings fall into place,
Maps sing tidy paths — I bound with gentle grace.

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Minor optimizations' is vague and generic. While it broadly relates to the changes, it lacks specificity about the actual optimizations made (string cache size increase and map[string]string decoding optimization).	Consider a more specific title like 'Optimize string cache and map[string]string decoding' to clearly convey the primary changes to reviewers.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch greg/perf

---

No actionable comments were generated in the recent review. 🎉

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2c81816 and 3f8cff5.

📒 Files selected for processing (6)

• CHANGELOG.md (1 hunks)
• internal/decoder/data_decoder.go (1 hunks)
• internal/decoder/decoder.go (1 hunks)
• internal/decoder/reflection.go (3 hunks)
• internal/decoder/reflection_test.go (1 hunks)
• internal/decoder/string_cache.go (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

internal/decoder/reflection_test.go (1)

internal/decoder/reflection.go (1)

• New (26-30)

internal/decoder/data_decoder.go (2)

mmdbdata/type.go (2)

• KindPointer (25-25)
• KindString (26-26)

internal/mmdberrors/errors.go (1)

• NewInvalidDatabaseError (24-26)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: Build (1.24, windows-latest)
• GitHub Check: Build (1.24, windows-latest)

🔇 Additional comments (7)

internal/decoder/string_cache.go (1)

17-17: LGTM! Cache size increase improves concurrent performance.

The 8x increase from 512 to 4096 entries should significantly reduce cache thrashing under higher concurrency. This increases memory usage to approximately 64KB per decoder instance (up from ~8KB), which is reasonable for the performance benefit.

CHANGELOG.md (1)

3-8: LGTM! Changelog accurately documents the performance improvements.

The changelog entry clearly describes both optimizations:

• Increased internal string cache size to reduce cache thrashing
• Fast path for map[string]string decoding

internal/decoder/decoder.go (1)

66-74: LGTM! Cleaner implementation using the new decodeStringValue method.

The refactored ReadString() now uses a single call to decodeStringValue instead of a multi-step process, making the code more concise and maintainable. The new implementation correctly handles pointer-following and offset management.

internal/decoder/reflection_test.go (1)

224-322: LGTM! Comprehensive test coverage for map[string]string decoding.

The tests cover all important scenarios:

• Empty and populated maps
• Merging and overwriting behavior
• Nil map initialization
• Error handling (non-string value)
• Nested structures to verify fast path is correctly applied

The test structure is clear and maintainable.

internal/decoder/reflection.go (3)

405-408: LGTM! Good organization grouping related type constants.

The type variables for sliceType and mapStringStringType are now grouped together, improving code organization and readability.

---

533-536: LGTM! Clean fast path dispatch for map[string]string.

The type check correctly identifies map[string]string and routes to the optimized decodeMapStringString method, falling back to the generic decodeMap for other map types. This provides significant performance benefits by avoiding reflection overhead for the common map[string]string case.

---

746-775: LGTM! Efficient fast path implementation for map[string]string.

The decodeMapStringString method efficiently handles the common case:

• Initializes nil maps with appropriate size hint
• Uses decodeStringValue for both keys and values (consistent with the new string decoding path)
• Correctly wraps value errors with map key context for debugging
• Properly tracks offsets throughout

The type assertion on line 755 is safe because of the type check in unmarshalMap (line 533). This optimization should significantly reduce allocations and improve performance for map[string]string decoding.