Optimize object key lookup through reducing String initialization#52
Optimize object key lookup through reducing String initialization#52
Conversation
Achieved by hashing the string (with FNV1) and storing the hash inline in the description
There was a problem hiding this comment.
Pull request overview
This PR optimizes object key lookup performance by storing pre-computed FNV-1a hashes alongside object keys in the JSON description structure. This allows the decoder to skip expensive memcmp operations when hash comparisons fail, achieving faster key lookups through early rejection.
Changes:
- Added FNV-1a hash computation during JSON parsing for object keys, stored inline in the description (13 bytes vs 9 bytes)
- Implemented hash-based comparison in keyOffset lookup to skip memcmp when hashes don't match
- Added sequential access hint optimization for decoder to improve performance on ordered key access
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| Sources/_NIOJSON/Core/JSONDescription.swift | Added FNV-1a hash function, objectKeyFound handler, and hash-based key lookup optimization with hint support |
| Sources/_NIOJSON/Core/JSONDescription+Constants.swift | Added constants for object key index length (13 bytes) and hash offset (9 bytes) |
| Sources/_NIOJSON/Types/JSONObject.swift | Added handling for objectKey types in value conversion, returning nil |
| Sources/_NIOJSON/Types/JSONArray.swift | Added fatalError for unexpected objectKey types in arrays |
| Sources/_NIOJSON/Codable/JSONDecoder.swift | Added lastKeySearchHint tracking and refactored key lookup to use valueOffset helper with hash optimization |
| Sources/_JSONCore/Parser/JSONTokenizerDestination.swift | Added objectKeyFound protocol method with default implementation for backward compatibility |
| Sources/_JSONCore/Parser/JSONParser+Parsing.swift | Added scanObjectKey function that computes FNV-1a hash during parsing |
| Sources/_JSONCore/Errors.swift | Updated TypeConversionError to use two generic parameters and made it Sendable |
| Package.swift | Added IkigaJSONCore library product and StrictConcurrency feature for _JSONCore target |
Comments suppressed due to low confidence (1)
Sources/_NIOJSON/Core/JSONDescription.swift:1
- The FNV-1a hash computation is duplicated in two places: the fnv1aHashString function (lines 9-15) and inline in scanObjectKey (lines 330-334). This code duplication creates a maintenance burden. Consider extracting a shared implementation or calling fnv1aHashString with the appropriate buffer slice.
import Foundation
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Key length is currentIndex - 1 (excluding opening quote, currentIndex points to closing quote) | ||
| let keyLength = currentIndex - 1 | ||
| var hash: UInt32 = 2166136261 // FNV offset basis | ||
| for i in 1...keyLength { |
There was a problem hiding this comment.
For empty keys (when keyLength is 0), the range 1...0 is invalid and will cause a runtime error. The loop should use ..< instead of ... or guard against empty keys. Change to for i in 1..<(keyLength + 1) or for i in 1...max(1, keyLength) to handle the empty key case safely.
| for i in 1...keyLength { | |
| for i in 1..<(keyLength + 1) { |
There was a problem hiding this comment.
@copilot make a unit test demonstrating this potential bug
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Joannis <1951674+Joannis@users.noreply.github.com>
Co-authored-by: Joannis <1951674+Joannis@users.noreply.github.com>
Co-authored-by: Joannis <1951674+Joannis@users.noreply.github.com>
Add unit test for lastKeySearchHint inheritance in nested decoders
Co-authored-by: Joannis <1951674+Joannis@users.noreply.github.com>
…intReset test Co-authored-by: Joannis <1951674+Joannis@users.noreply.github.com>
Fix empty object key crash in hash computation
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
Sources/_NIOJSON/Codable/JSONDecoder.swift:370
- The subDecoder method doesn't reset lastKeySearchHint when creating a child decoder for nested objects. This causes the child decoder to inherit the parent's hint value, which points to an invalid offset in the child's index space. The hint should be reset to Constants.firstArrayObjectChildOffset for nested decoders. While the test at line 1634 documents this as working correctly due to wraparound logic, it's still an optimization inefficiency that causes unnecessary wraparound searches.
func subDecoder(offsetBy offset: Int) -> _JSONDecoder {
let subDescription = self.description.unsafeReadOnlySubDescription(offset: offset)
return _JSONDecoder(description: subDescription, codingPath: _codingPath, pointer: pointer, settings: settings)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Achieved by hashing the string (with FNV1) and storing the hash inline in the description