fix: Add StringTable and refactor BinaryFormat #8243
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Performance ReportDaily Performancexychart-beta
title Files Per Second by Day
y-axis Files per Second
x-axis Date [Nov-29, Nov-30, Dec-2, Dec-8, Dec-9, Dec-10, Dec-13, Dec-14, Dec-15, Dec-16, Dec-20, Dec-22, Dec-23, Dec-24, Dec-27, Dec-28, Dec-29]
bar [158.58, 159.25, 158.41, 158.24, 160.26, 160.13, 162.46, 160.30, 159.32, 159.99, 160.32, 171.41, 173.30, 173.54, 170.88, 173.10, 172.63]
line [6.01, 6.16, 6.12, 6.00, 6.03, 6.11, 5.69, 6.12, 5.85, 6.27, 5.94, 6.19, 6.23, 6.06, 5.80, 6.19, 6.05]
line [76.76, 75.06, 80.27, 76.65, 75.94, 76.12, 76.57, 75.62, 71.86, 75.70, 72.52, 79.52, 79.75, 79.57, 78.19, 78.53, 79.80]
line [62.90, 65.22, 65.65, 62.39, 60.36, 64.84, 65.11, 62.93, 65.98, 63.02, 64.70, 64.39, 64.92, 64.92, 65.76, 63.97, 65.07]
line [71.50, 72.49, 68.01, 65.32, 69.78, 72.32, 70.99, 69.49, 68.20, 68.29, 71.18, 72.28, 72.81, 72.16, 72.22, 72.29, 70.42]
line [39.45, 38.80, 39.93, 37.87, 36.13, 38.10, 36.47, 38.38, 37.81, 37.32, 38.13, 39.99, 39.24, 39.22, 39.13, 37.99, 37.85]
line [73.38, 73.35, 76.02, 71.59, 73.55, 75.54, 73.95, 75.07, 76.77, 72.26, 69.84, 77.03, 74.27, 74.27, 73.83, 75.96, 73.77]
line [50.21, 51.36, 49.95, 51.18, 48.94, 51.93, 50.78, 49.91, 50.66, 50.34, 50.02, 51.61, 51.21, 51.38, 49.64, 48.60, 49.08]
line [97.20, 94.57, 96.80, 90.09, 95.39, 94.62, 99.56, 95.79, 93.99, 95.03, 91.66, 95.50, 94.32, 94.29, 94.73, 94.34, 95.00]
line [202.79, 200.07, 197.01, 195.46, 191.88, 199.25, 200.18, 192.55, 195.85, 196.96, 197.31, 203.68, 215.88, 203.82, 209.47, 208.55, 207.08]
line [109.21, 117.71, 115.04, 114.43, 117.93, 116.17, 113.85, 113.19, 111.86, 113.23, 116.22, 119.95, 116.76, 117.75, 118.71, 118.58, 117.65]
line [18.13, 19.36, 18.09, 18.49, 18.92, 19.04, 18.41, 18.71, 17.35, 18.44, 18.66, 19.39, 19.24, 18.95, 18.61, 19.20, 18.73]
line [23.50, 23.32, 21.90, 22.93, 23.26, 21.71, 22.48, 22.26, 24.16, 21.86, 22.99, 23.86, 22.95, 22.15, 22.59, 22.93, 22.51]
line [131.34, 132.15, 133.58, 127.82, 130.48, 134.87, 129.94, 136.83, 140.59, 128.00, 132.13, 140.04, 140.36, 136.98, 135.57, 135.50, 136.47]
line [28.98, 28.47, 31.13, 29.50, 29.81, 30.70, 31.34, 29.78, 30.86, 29.89, 30.20, 33.22, 32.90, 33.40, 32.96, 33.38, 33.08]
line [14.45, 14.21, 13.55, 14.37, 14.53, 14.61, 14.25, 14.42, 14.66, 14.20, 14.14, 14.58, 14.66, 13.81, 14.51, 13.39, 13.55]
line [113.61, 117.05, 115.87, 112.86, 110.11, 114.79, 114.58, 111.22, 109.33, 106.79, 113.91, 116.30, 113.89, 119.27, 121.25, 119.32, 120.20]
line [43.85, 45.20, 42.00, 46.02, 43.67, 43.68, 45.02, 45.75, 43.43, 44.94, 44.85, 45.35, 43.98, 45.75, 48.03, 44.47, 44.56]
line [117.56, 113.82, 115.04, 119.17, 112.82, 116.99, 116.61, 109.61, 115.93, 113.66, 115.36, 122.68, 121.26, 119.81, 119.36, 122.21, 117.72]
line [317.57, 310.02, 296.47, 313.58, 317.30, 295.96, 306.54, 308.67, 318.29, 308.47, 312.68, 311.01, 311.87, 322.19, 317.56, 317.24, 318.95]
line [125.68, 123.73, 126.68, 128.02, 124.91, 128.75, 126.25, 125.99, 122.54, 125.77, 120.95, 129.07, 131.35, 128.71, 129.73, 130.21, 126.40]
line [190.53, 196.53, 193.86, 202.32, 202.37, 184.12, 199.02, 199.50, 195.58, 198.56, 202.04, 205.76, 202.04, 203.32, 201.04, 202.44, 202.80]
line [120.89, 122.51, 125.41, 124.96, 118.90, 125.77, 122.50, 124.97, 121.63, 124.44, 114.40, 117.66, 113.42, 116.57, 120.86, 116.04, 114.91]
line [194.09, 191.43, 192.21, 203.69, 197.75, 183.65, 201.42, 192.31, 202.71, 196.90, 201.49, 199.06, 199.67, 194.62, 190.28, 203.79, 205.03]
line [220.14, 219.09, 206.67, 213.42, 215.72, 212.77, 206.57, 216.35, 210.81, 207.51, 213.81, 235.09, 232.12, 236.13, 235.71, 228.72, 232.22]
line [236.55, 228.65, 226.03, 235.13, 236.77, 244.11, 232.83, 233.41, 237.39, 235.87, 236.73, 241.85, 239.59, 240.67, 245.15, 245.04, 243.25]
line [70.33, 71.28, 70.66, 70.99, 71.64, 73.38, 70.10, 71.99, 68.96, 71.11, 72.00, 74.62, 76.21, 75.66, 76.28, 80.16, 76.08]
line [175.76, 181.77, 182.47, 179.95, 179.31, 188.38, 180.77, 185.77, 186.35, 184.33, 182.33, 191.53, 189.75, 189.98, 177.07, 192.83, 188.99]
line [192.30, 195.25, 190.71, 193.15, 186.55, 193.19, 192.23, 185.43, 191.26, 188.26, 189.10, 196.56, 199.42, 199.83, 198.90, 201.82, 196.19]
line [139.07, 144.20, 143.55, 138.09, 144.82, 146.07, 144.49, 139.02, 141.78, 142.36, 140.50, 151.95, 150.33, 152.02, 150.72, 154.90, 154.26]
line [183.30, 171.17, 189.43, 187.46, 185.27, 181.05, 182.93, 185.74, 173.94, 185.68, 184.19, 188.28, 188.11, 196.54, 189.53, 183.55, 192.10]
line [211.90, 198.01, 215.07, 216.21, 215.29, 206.31, 212.61, 210.51, 204.56, 210.18, 211.34, 200.66, 223.22, 223.16, 220.54, 207.84, 221.58]
line [150.86, 144.17, 148.12, 146.38, 144.41, 148.76, 149.26, 148.44, 146.25, 144.57, 148.51, 160.26, 155.20, 157.30, 154.76, 157.76, 155.18]
line [87.05, 85.49, 84.05, 80.02, 85.30, 82.71, 84.32, 82.91, 77.95, 81.87, 81.89, 89.30, 90.84, 89.17, 86.07, 86.04, 90.72]
line [293.25, 299.40, 309.39, 295.84, 293.27, 283.38, 296.25, 298.40, 299.27, 280.90, 293.45, 278.89, 296.00, 294.23, 289.77, 295.71, 299.08]
line [352.03, 379.23, 367.49, 374.69, 369.34, 374.83, 359.05, 350.76, 385.22, 356.23, 361.41, 363.52, 371.74, 360.07, 349.58, 358.00, 363.57]
line [232.03, 221.81, 240.23, 229.14, 227.27, 231.37, 242.80, 219.56, 233.62, 231.32, 236.32, 236.26, 240.94, 253.96, 249.85, 237.45, 245.35]
line [229.81, 229.18, 221.75, 237.38, 239.25, 224.99, 230.85, 236.16, 237.46, 237.06, 237.81, 238.36, 240.26, 242.42, 235.97, 234.05, 232.74]
line [32.17, 27.01, 31.76, 31.99, 31.99, 30.84, 32.08, 31.89, 31.67, 31.01, 31.00, 31.79, 31.95, 32.72, 33.00, 31.56, 31.10]
line [162.13, 167.55, 165.94, 166.49, 162.27, 148.64, 164.74, 167.10, 169.90, 168.25, 171.20, 165.98, 169.49, 175.43, 173.51, 166.13, 172.91]
line [168.29, 166.84, 155.06, 164.08, 161.66, 171.02, 169.99, 161.97, 169.28, 168.79, 164.45, 163.90, 173.08, 173.52, 171.67, 165.07, 169.92]
line [98.38, 95.87, 95.57, 93.03, 94.71, 95.93, 94.05, 97.42, 87.40, 93.20, 92.02, 98.77, 95.06, 94.41, 95.55, 97.07, 94.61]
line [147.45, 147.90, 147.19, 142.70, 144.08, 143.97, 142.44, 148.72, 148.74, 148.66, 145.81, 145.83, 151.65, 150.82, 149.41, 147.54, 147.36]
line [339.55, 340.91, 343.35, 333.69, 321.17, 305.21, 329.78, 336.28, 339.96, 335.03, 330.81, 346.52, 335.23, 343.48, 342.97, 326.49, 352.75]
line [47.22, 46.33, 44.42, 44.87, 45.90, 44.67, 46.00, 48.05, 47.07, 45.85, 42.75, 47.52, 47.21, 45.81, 45.88, 45.34, 46.82]
line [23.09, 24.60, 24.46, 23.99, 23.97, 23.56, 24.27, 23.97, 21.89, 24.77, 24.75, 25.98, 24.78, 24.02, 24.07, 23.90, 24.45]
line [206.34, 190.41, 205.06, 206.03, 204.37, 194.91, 205.14, 200.36, 207.64, 202.85, 200.31, 210.29, 210.04, 207.76, 200.78, 205.77, 201.30]
line [158.52, 161.43, 161.03, 163.37, 162.01, 157.69, 162.41, 162.92, 155.61, 157.24, 162.25, 174.81, 169.46, 179.44, 172.82, 176.92, 176.06]
line [158.17, 155.13, 153.99, 150.95, 152.99, 153.72, 160.91, 157.90, 164.74, 161.04, 164.69, 164.20, 175.57, 172.74, 173.87, 177.24, 171.60]
line [116.12, 108.62, 108.44, 104.81, 110.68, 102.65, 110.81, 107.51, 105.11, 109.29, 110.23, 118.20, 116.66, 113.51, 116.56, 110.18, 116.75]
line [135.37, 140.89, 136.51, 139.19, 142.56, 144.70, 146.06, 142.91, 137.61, 141.27, 139.39, 164.22, 164.25, 165.65, 160.55, 167.52, 163.60]
Time to Process Files
Note:
Files per Second over Time
Data Throughput
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the BinaryFormat classes by moving them from TrieBlob/binaryFormat.ts to a new shared binary module, adds a new StringTable implementation for efficient string storage, and adds generic type parameters <ArrayBuffer> to typed array declarations throughout the codebase for improved type safety.
- Refactored binary format classes to a shared module with enhanced functionality including Uint16 support and field overloading
- Added new StringTable class for compact storage and retrieval of strings with substring deduplication
- Standardized typed array declarations with explicit
<ArrayBuffer>generic parameters across the codebase
Reviewed changes
Copilot reviewed 19 out of 21 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
packages/cspell-trie-lib/src/lib/io/decode.ts |
Added <ArrayBuffer> generic parameters to typed array type declarations |
packages/cspell-trie-lib/src/lib/decodeTrie.ts |
Added <ArrayBuffer> generic parameters to typed array type declarations |
packages/cspell-trie-lib/src/lib/binary/index.ts |
New barrel export for binary format classes and utilities |
packages/cspell-trie-lib/src/lib/binary/hexDump.ts |
New utility function for generating hex dump representations of byte arrays |
packages/cspell-trie-lib/src/lib/binary/binaryFormat.ts |
Refactored binary format classes with added Uint16 support, field overloading, and enhanced documentation |
packages/cspell-trie-lib/src/lib/binary/binaryFormat.test.ts |
Added tests for new Uint16 and string pointer functionality, and field overloading feature |
packages/cspell-trie-lib/src/lib/binary/__snapshots__/binaryFormat.test.ts.snap |
Updated snapshots to reflect changes in binary format structure (added alignment and byteSize fields) |
packages/cspell-trie-lib/src/lib/TrieData.ts |
Added <ArrayBuffer> generic parameter to encodeToBTrie return type |
packages/cspell-trie-lib/src/lib/TrieBlob/trieDataEncoder.ts |
Added <ArrayBuffer> generic parameters and updated to use new binary module import path |
packages/cspell-trie-lib/src/lib/TrieBlob/binaryFormat.ts |
Deleted - moved to binary/binaryFormat.ts |
packages/cspell-trie-lib/src/lib/TrieBlob/TrieBlobInfo.ts |
Added <ArrayBuffer> generic parameter to nodes array type |
packages/cspell-trie-lib/src/lib/TrieBlob/TrieBlobEncoder.ts |
Updated imports to use new binary module and added type aliases |
packages/cspell-trie-lib/src/lib/TrieBlob/TrieBlob.ts |
Updated to use type aliases with <ArrayBuffer> generic parameters throughout |
packages/cspell-trie-lib/src/lib/TrieBlob/TrieBlob.test.ts |
Updated hexDump import to use new binary module |
packages/cspell-trie-lib/src/lib/TrieBlob/FastTrieBlob.ts |
Added <ArrayBuffer> generic parameters to typed array types |
packages/cspell-trie-lib/src/lib/StringTable/__snapshots__/StringTable.test.ts.snap |
New snapshot for StringTable encoding test |
packages/cspell-trie-lib/src/lib/StringTable/StringTable.ts |
New StringTable implementation with builder pattern for efficient string storage |
packages/cspell-trie-lib/src/lib/StringTable/StringTable.test.ts |
New tests for StringTable builder, encoding, and decoding |
packages/cspell-trie-lib/api/api.d.ts |
Updated API type definitions to include <ArrayBuffer> generic parameters |
packages/cspell-lib/src/lib/SpellingDictionary/DictionaryController/DictionaryLoader.ts |
Added <ArrayBuffer> generic parameter to Reader interface |
packages/cspell-dictionary/src/SpellingDictionary/SpellingDictionaryFromTrie.ts |
Added <ArrayBuffer> generic parameter to function parameter type |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Jason Dent <Jason3S@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Jason Dent <Jason3S@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 21 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Jason Dent <Jason3S@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 21 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #getPtrData(element: DataElementWithRef): U8Array { | ||
| const formatElement = element.ref; | ||
| assert(formatElement.type === 'ptr+size', `Field is not a ptr: ${element.name} (${formatElement.type})`); | ||
| const view = new DataView<ArrayBuffer>(element.data.buffer, element.data.byteOffset, element.data.byteLength); |
There was a problem hiding this comment.
Inconsistent use of DataView type parameter. Line 729 uses new DataView<ArrayBuffer>() while all other DataView instantiations in this file omit the type parameter. For consistency, either add the type parameter to all DataView usages or remove it from this one.
Suggested change
| const view = new DataView<ArrayBuffer>(element.data.buffer, element.data.byteOffset, element.data.byteLength); | |
| const view = new DataView(element.data.buffer, element.data.byteOffset, element.data.byteLength); |
| */ | ||
| getPtrUint8Array(name: string): U8Array { | ||
| const element = this.getDataElement(name); | ||
| assert(element.ref.type === 'ptr+size', `Field is not a ptr_and_length: ${name}`); |
There was a problem hiding this comment.
The error message uses "ptr_and_length" but the actual FormatType value is "ptr+size". The error message should be updated to match the actual type name for consistency.
| */ | ||
| getPtrString(name: string): string { | ||
| const element = this.getDataElement(name); | ||
| assert(element.ref.type === 'ptr+size', `Field is not a ptr_and_length: ${name}`); |
There was a problem hiding this comment.
The error message uses "ptr_and_length" but the actual FormatType value is "ptr+size". The error message should be updated to match the actual type name for consistency.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.