-
Notifications
You must be signed in to change notification settings - Fork 311
HPCC-33560 Reuse the compressor when building hybrid indexes #20770
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: candidate-10.0.x
Are you sure you want to change the base?
Conversation
|
Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-33560 Jirabot Action Result: |
|
First commit is part of a separate PR. |
There was a problem hiding this 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 refactors the hybrid index compression implementation to reuse compressor instances instead of creating new ones for each node, improving efficiency during index building. The changes consolidate the HybridIndexCompressor class into the jhblockcompressed module and eliminate the redundant BlockCompressedIndexCompressor class.
Key Changes:
- Moved HybridIndexCompressor from keybuild.cpp to jhblockcompressed.hpp/cpp for better code organization
- Modified CBlockCompressedBuildContext to store and initialize a reusable ICompressor instance
- Updated CBlockCompressedWriteNode to use the shared compressor instead of creating new instances
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| system/jhtree/keybuild.cpp | Removed HybridIndexCompressor class definition (moved to jhblockcompressed module) |
| system/jhtree/jhblockcompressed.hpp | Added jhinplace.hpp include, modified CBlockCompressedBuildContext to support compressor reuse, and moved HybridIndexCompressor here |
| system/jhtree/jhblockcompressed.cpp | Implemented compressor reuse by storing a single ICompressor in CBlockCompressedBuildContext and moved/refactored HybridIndexCompressor implementation |
|
|
||
| struct CBlockCompressedBuildContext | ||
| { | ||
| public: |
Copilot
AI
Jan 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The struct has two consecutive public access specifiers. The first public label at line 84 is redundant and should be removed, as structs have public access by default and there's another public label immediately following at line 87.
| public: |
Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>
Type of change:
Checklist:
Smoketest:
Testing: