Repository navigation
Initial support for lhc4codec - #23391
apeters1971 wants to merge 5 commits into
Conversation
jblomer
left a comment
There was a problem hiding this comment.
Thank you!
In principle looks good to me!
Regarding commits: can you split the first commit in three:
- addition of the new algorithm
- modifications to RNTuple
- the new binaries (
[rnutple|tree]2rntuple)
Can you submit the second commit as a separate PR to the master branch? I'll then merge this and rebase the lhc4codec branch.
The failure dumps are ok for now but should be removed eventually, I think. If easily possible, perhaps that functionality can be guarded by a define.
| const auto subfields = c.fRecordField->GetConstSubfields(); | ||
| const auto &offsets = c.fRecordField->GetOffsets(); | ||
| R__ASSERT(subfields.size() == c.fLeafBranchIndexes.size()); | ||
| c.fPackedLeaves.clear(); |
There was a problem hiding this comment.
assert(c.fPackedLeaves.empty())?
| if (fTypeAlias == "Double32_t") { | ||
| if (options.GetEnableColumnEncoding() && options.GetCompression() != 0) | ||
| SetColumnRepresentatives({{ROOT::ENTupleColumnType::kSplitReal32}}); | ||
| else | ||
| SetColumnRepresentatives({{ROOT::ENTupleColumnType::kReal32}}); | ||
| } |
There was a problem hiding this comment.
I guess this could also be a separate fix to master? And here, it should use usePlainColumns.
| fSealedPages.clear(); | ||
| } | ||
|
|
||
| void ROOT::Internal::RPageSinkBuf::RColumnBuf::ValidateReadyToCommit(bool requireChecksum) const |
There was a problem hiding this comment.
This seems unrelated to the new compression algorithm. What is the motivation to add this?
| static int R__LHC4Codec = static_cast<int>(lhc4codec::Codec::Beam); | ||
| static int R__LHC4Filters = 0; | ||
| static int R__LHC4FilterFallback = 1; | ||
| static int R__LHC4FilterRle = 1; | ||
| static int R__LHC4FilterDict = 1; | ||
| static int R__LHC4AutoMinGainPct = 1; | ||
| static int R__LHC4AutoMaxLevel = 0; | ||
| static unsigned R__LHC4AutoCodecs = kLHC4AutoCodecsRoot; | ||
| static int R__LHC4LastCxLevel = -1; |
There was a problem hiding this comment.
Perhaps add a TODO: At some point, we need to decide on settings or make it part of the level option.
754890a to
2c33654
Compare
Double32_t always forced split encoding, even with compression off. Use kReal32 when there is nothing to encode against.
Add RImportReport so Import() exposes compressed page payload, uncompressed pages, and the destination TFile size after the writer commits.
GetConstSubfields() was allocating a new vector on every TTree entry, which made tree2ntuple and other imports much slower than they should be. Caching the leaf-count layout once removes that bottleneck and makes the conversion run much quicker.
2c33654 to
597c9f0
Compare
Wire kLHC4 through ROOT zip, with builtin or system lhc4codec (pinned to c433157), and the Flash/Beam/Prism/Mosaic/Oracle/Crystal/Laser/Grating/Auto backends behind process-wide R__SetLHC4* settings for the codec, the byte filters, the Auto gain budget and codec set, and the pretrained Crystal profiles. Failure dumps compile only with R__LHC4_FAILURE_DUMP. RNTuple: EnableColumnEncoding lets pages be stored without split/zigzag/delta when the LHC4 filters do that work; Double32_t follows usePlainColumns. The page sink keeps a thread-local Crystal profile per column type so parallel page compression does not share one setting; lhc4_crystal5 stays the dynamic compressor and the pretrained per-column models are lhc4_crystal_profile5. Zip helpers and tests cover the new algorithm. Conversion tools unpack ROOT split encodings before an LHC4 recode. Tools: tree2ntuple converts a TTree into one RNTuple per compression variant (native_zstd, native_lzma, lhc4_zstd, lhc4_bzip3, lhc4_beamN, lhc4_prismN, lhc4_mosaicN, lhc4_crystalN, lhc4_crystal_profileN, lhc4_oracleN, lhc4_flashN, lhc4_laserN, lhc4_grating, lhc4_auto) with --variants, --level, --no-out, --verify, --jobs, --page-size, --bench-percent and the --auto-* settings, and prints size, page ratio, gain against native_zstd, the filter statistics and the Auto codec mix. rntuple2rntuple re-encodes an RNTuple the same way. Both carry the read benchmarks of RNTupleBench.hxx (--bench, rntuple2rntuple --bench-only): pure page decompression, LoadEntry on one thread and LoadEntry with IMT. rntupleinspect judges page formats; rntupletryzip recompresses pages in memory (native probes re-apply ROOT encodings; --threads races codecs and filters).
Train one static Crystal model per column from its first pages and store it once in the column descriptor instead of shipping a ~1.5 KiB model with every page. Format: kFeatureFlag_CompressionContext (1) in the header feature flags and column record flag 0x04 carrying an RCompressionContext (algorithm, type, size, payload); RNTupleDescriptor and the serializer read and write it, with a unit test. Writer: RNTupleWriteOptions::SetEnableLearnedProfiles, the training sample size (256 KiB per column, capped across columns) and the train level. The persistent sink buffers a column's uncompressed pages until the sample is complete, trains and registers the model, seals later pages with that profile id, trains the still-pending columns before the header is committed, and defers the header while profiles are enabled. RPageSinkBuf forwards the profile id to the parallel zip tasks. Pages written before the sample is full stay dynamic; a page the profile loses escapes to a dynamic block in the codec. Reader: RPageSource registers the models from the descriptor on attach. core/lhc4codec: R__LHC4TrainProfileModel[Level], R__LHC4RegisterProfileModel, R__LHC4ExportProfileModel, a thread-local call profile id, and the Crystal profile block / escape counters in the compression statistics. Tools: lhc4_crystal_learned5/7/9 variants and the profile escape report in tree2ntuple and rntuple2rntuple. tree/ntuple/doc/LearnedCompressionProfiles.md documents the format, the codec side (precomputed decode tables, drifting-column transforms, per-page lane modes on profile blocks) and the measurements: at 1 MiB pages the learned file is 0.4% larger than dynamic Crystal, decodes 3-4% faster and halves the encode CPU; at 16 KiB pages it is 2.4% smaller and writes five times faster.
597c9f0 to
0cbd2e3
Compare
| * For the licensing terms see $ROOTSYS/LICENSE. * | ||
| * For the list of contributors see $ROOTSYS/README/CREDITS. * | ||
| *************************************************************************/ | ||
|
|
There was a problem hiding this comment.
Side note: the headers of type Zip*.h will become private in ROOT 6.46, see https://github.com/root-project/root/pull/23271/files#diff-db560c023d52609eb4f91b7110ac6ecca1ef364ee77c669ab5bb4f22075d8bfa
The goal is that the header RZip.h is the public entry point for external users, in the .cxx one will then include Zip*.h
| #include <RConfigure.h> | ||
|
|
||
| #ifdef R__HAS_LHC4CODEC | ||
| #include <ZipLHC4.h> |
There was a problem hiding this comment.
Could this go instead over public RZip.h interface?
The RZip.h interface can be extended with new functions if needed and then RZip.cxx can forward the calls to the private ZipLHC4 header.
| #include "TInterpreter.h" | ||
|
|
||
| #ifdef R__HAS_LHC4CODEC | ||
| #include "ZipLHC4.h" |
There was a problem hiding this comment.
same question here, etc.
| set(LHC4CODEC_INSTALL_HELP | ||
| "lhc4codec was not found. Install lhc4codec-devel / brew tap apeters/lhc4codec, set -DLHC4CODEC_ROOT, or use -Dbuiltin_lhc4codec=ON.") | ||
| endif() | ||
| message(FATAL_ERROR "${LHC4CODEC_INSTALL_HELP}") |
There was a problem hiding this comment.
| message(FATAL_ERROR "${LHC4CODEC_INSTALL_HELP}") | |
| message(SEND_ERROR "${LHC4CODEC_INSTALL_HELP}") | |
| list(APPEND MISSING_PACKAGES 'lhc4codec') | |
| list(APPEND HOTFIX_BUILD_FLAGS -Dbuiltin_lhc4codec=ON) |
This Pull request:
Changes or fixes:
Checklist:
This PR fixes #