Repository navigation
Share the CLI's fast pipeline with Python and C through the library - #1531
Open
ebursztein wants to merge 13 commits into
Open
ebursztein wants to merge 13 commits into
ebursztein wants to merge 13 commits into
Conversation
Coverage Report for CI Build 37947211612Coverage increased (+0.005%) to 96.841%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions1 previously-covered line in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
1 of 7 tasks
The GPU qualification test only compiles on macOS or with CUDA, so CI did not see that it still used Features.0 and the old FileType::convert signature. It now passes the options, with rules off since the reference outputs are the model's alone, and maps RulesVeto as unreachable.
Features extracted with use_rules = false recorded an empty list of matching rules, so a session with rules on vetoed every prediction whose rules have no false negatives: real WAV, PSD, Parquet and XLSX files came out unknown. Features now record None when the rules did not run, and the veto needs evidence that they ran and did not match.
Errors were the parser's debug output with byte spans, such as Span(13..13). Custom rules (next commit) make these errors user-facing, so each now reads "line L, column C: message".
Options::custom_rules holds compiled Rules (an Arc, so options stay cheap to clone), built with Rules::compile or Rules::from_files, or set with Builder::with_custom_rules. Extraction runs them first, then the built-in rules when use_rules is set, then the model: - custom rules identify a file when the ones that match agree on one content type; - built-in rules identify it when no custom rule matched and the ones that match agree; - otherwise the model decides, and a rule set that ran vetoes a content type it claims to never miss (every enforced rule of it is class "full") when none of its rules matched. Both sets share one read of a zip archive's tail. Features keep what matched and which rule sets ran, so the veto follows the rules used at extraction rather than the session's options. Custom rules use the validation of magika-rules, must label Magika content types, and must enforce at least one rule.
--rules-file (repeatable) passes custom rules to the library, which checks them before the built-in rules whatever --rules is; --rules-check validates them and prints the content types they identify. tests_data/rules/custom.yar enforces the PNG signature, which the built-in rules leave to the model, so the CLI, Python and C tests can share one custom rule.
Magika(rules=...) takes YARA text and Magika(rules_files=[...]) paths; invalid rules raise ValueError naming the line, file or rule. The tests share tests_data/rules/custom.yar with the CLI and check identify_path, identify_paths and identify_bytes, the veto of a full rule that does not match, and the errors.
magika_rules_new compiles YARA text into an opaque MagikaRules, writing the error message into a caller buffer on MAGIKA_STATUS_INVALID_RULES, and magika_rules_free releases it. MagikaOptions gains custom_rules; the options keep their own reference, so the caller may free the rules once the call returns. The test identifies the PNG sample by rules alone only with tests_data/rules/custom.yar. The header is edited by hand, as cbindgen would write it.
The CLI's orchestration moves into the library so that Python and C can share it, behind a pipeline feature that is off by default. Both helpers use only the public API, which stays the way to write a pipeline specialized to where Magika is embedded. - Engine prepares the CPU and, unless the CPU is required, the GPU on background threads, so creating one returns at once. Each thread identifies through an EngineSession, which keeps one session per backend and sends full batches to the GPU once it is ready (always, when it is required). Options are given per call, so one engine serves any options. - Pipeline is the CLI's walk, read, batch, inference and reorder stages: identify_paths returns each path with its result in walk order, and dropping the iterator stops the run. While a GPU is prepared, two thirds of the CPU inference threads start, and the rest only if the GPU fails. - The _trace feature reports per-stage busy and waiting time, as the CLI's did. Builder::backend() exposes the selected backend, which Engine needs.
The CLI keeps its flags, standard input and printing, and its walk, read, batch, inference and reorder stages, backends.rs and _trace report are now the library's Engine and Pipeline (_trace forwards to magika/_trace). Medians of 10 interleaved runs over evaluation dataset files, before and after, on an Apple M-series machine: 1 file 11.9 / 11.9 ms, 10 files 18.1 / 18.1 ms, 100 files 40.5 / 40.6 ms, 1,000 files 140.4 / 140.0 ms, 3,000 files 260.4 / 259.8 ms.
The Python extension shares one Engine per process instead of one Runtime, with an EngineSession per thread, and identify_paths runs the library Pipeline instead of reading files one by one into a single batch. Magika() no longer waits for the GPU. The Python API and results are unchanged. Medians of 9 cold processes, before and after, over evaluation dataset files: import and one identify_path 131.9 / 30.7 ms; identify_paths over 100 files 146.8 / 56.0 ms, over 3,000 files 596.5 / 286.5 ms.
magika_engine_new starts the library Engine from the same MagikaRuntimeOptions as a runtime and returns before the model is ready; magika_engine_identify_paths runs the library Pipeline over many paths, with a result and a status per path; magika_engine_free releases it. The existing runtime, session and features functions are unchanged; runtime and engine creation now share the builder setup, and per-path errors use the same status mapping as other calls. The header is edited by hand, as cbindgen would write it.
This branch has not been deployed
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
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.
Part of #1537.
Stacked on #1530 (custom rules), itself on #1529: review only the last 4 commits, from
Add Engine and Pipeline helpers behind the pipeline feature. Once those merge, this branch is rebased.Why
The CLI had its own fast orchestration: parallel reading, batching, and identifying on the CPU while the GPU is prepared. Python and C didn't share it. Python's
Magika()waited about 95 ms for the GPU before its first answer, and itsidentify_pathsread every file on one thread.This PR moves that orchestration into the library as two helpers behind a
pipelinefeature that is off by default. The real API doesn't change:FeaturesOrRuled::extract_*andSession::identify_features_batchare still the way to write a pipeline specialized to where Magika is embedded. Both helpers are written against that public API only, so they also serve as examples of it.The helpers (
magika::pipeline)Engine:Engine::new(builder)prepares the CPU runtime on a background thread, and then (unlessBackend::Cpu) the GPU runtime, so it returns at once.EngineSession(identify_file,identify_content,identify_features_batch), which holds one session per backend, spawned on first use. Full batches (8 or more) go to the GPU once it's ready, or always when it's required.backends.rs, moved.Pipeline:Pipeline::new(engine, options, PipelineOptions { recursive, stdin, batch_size, threads, readers }).identify_paths(paths)returns(path, Result<FileType>)in walk order. An outer error means the pipeline itself failed; dropping the iterator stops the run.read_dirtypes), readers, batcher, inference workers and reordering. Two thirds of the workers start while the GPU is prepared, and the rest only if it fails.DirectoryCycleis a typed error, replacing a string comparison.Builder::backend()returns the selected backend._tracefeature keeps the CLI's per-stage report.Front ends
Identify paths in the CLI with the library pipelinebackends.rs, the stages and the trace code go (+39 / −640 lines). The CLI'stest.shpasses as is. One deliberate change: with--backend=gpu, a short run now waits for the GPU instead of quietly identifying on the CPU.Identify in Python with the library pipelineEngineper process, anEngineSessionper thread, andidentify_pathsonPipeline.Magika()waits for the CPU runtime only, so a model failure is still reported at construction and a forked process doesn't inherit a preparation in progress. The Python API and results are unchanged.Identify paths in parallel from the C librarymagika_engine_new/magika_engine_freeandmagika_engine_identify_paths, with a result and a status per path. Existing functions are unchanged. The header is edited by hand, as cbindgen would write it.Numbers
Apple M-series, release builds, evaluation dataset files, interleaved with the code before this PR (#1530).
CLI (median of 10 runs). The gate was "no more than 5% slower at any size".
Python (median of 9 cold processes, including
import magika):Magika()then oneidentify_pathidentify_paths, 100 filesidentify_paths, 3,000 filesReview fixes
A separate review of this branch (design, safety, concurrency) checked that:
It found four issues, fixed here:
Magika()waits for the CPU runtime.--backend=gpuchange is in the changelog, the arrays' state on error is documented, and freeing an engine notes that its background preparation may still be running.Testing
tests_data/basicgives the same label asSession::identify_filefor every file, in sorted walk order;DirectoryCycle;Unsupported, and a directory isDirectorywithout recursion;test.sh: tests with--features=pipeline, and checks and lints with--features=_trace. The default build doesn't compile the module.test.sh: unchanged and passing, covering the three rules modes, custom rules, symlinks, missing files, non-UTF-8 names, directory cycles and broken pipes.run_quick_test_magika_module.pypass; ruff and mypy are clean.test.sh: passes under ASan and UBSan with gcc and clang, static and shared, including the engine with a missing path.rust/test.shpasses on stable and nightly (52 test suites). The final sync check only flagsmodel.probe.f32le, which regenerates differently on this Mac, as onmain.rust/changelog.shpasses.🤖 Generated with Claude Code