[11.x.x] Parallelize per-sample test-pack generation in PipelineTestPackWriter - #388
Open
hugohills-regnosys wants to merge 2 commits into
Open
hugohills-regnosys wants to merge 2 commits into
hugohills-regnosys wants to merge 2 commits into
Conversation
writeTestPackSamples() generated every sample in a test pack sequentially on a single thread, even though each sample is fully independent (own input file, own output file, own SampleModel). For a model with tens of thousands of samples across many test packs, this loop dominates update-expectations runtime (e.g. one BNPP pipeline: ~17,700 samples at ~39ms each = ~11.4 minutes, single-threaded). Verified thread-safety of everything already shared across samples before parallelizing: RosettaTypeValidator, WorkflowPostProcessor and its constituent PostProcessSteps, and ReferenceResolverProcessStep all create their mutable state fresh per call and only hold immutable config/factory fields, so concurrent invocation is safe. The one real hazard was the single javax.xml.validation.Validator built once per test pack and reused across all its samples - Validator is explicitly not thread-safe. PipelineFunctionRunnerProvider/Impl now take a Schema instead and call schema.newValidator() per run() invocation (cheap, and Schema is safe to share). TransformTestExtension updated to match. Also synchronize the ValidationSummariser callback per sample, since arbitrary implementations of that interface aren't guaranteed thread-safe. writeTestPackSamples' per-sample body is extracted into generateSample() and driven by inputSamplesForTestPack.parallelStream(); output is re-sorted by sample ID afterward as before, so file layout and generated config are unchanged - only the generation order and wall-clock time.
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
1 participant
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.
Backport of #11 onto the
11.x.xrelease line (which publishes ascom.regnosys:rosetta-testing, the artifact actually consumed by current model repos, distinct frommain'sorg.finos.rune-testing:rune-testingcoordinates). Cherry-picked cleanly with no conflicts.Summary
PipelineTestPackWriter.writeTestPackSamples()generated every sample in a test pack sequentially on a single thread, even though each sample is fully independent (own input file, own output file, ownSampleModel). For models with large test-pack suites this loop dominatesupdate-expectationsruntime.inputSamplesForTestPack.parallelStream(), with the per-sample body extracted into a newgenerateSample()method. Output is still re-sorted by sample ID afterward, so file layout and generated config JSON are unchanged.Thread-safety review
RosettaTypeValidator,WorkflowPostProcessorand its constituentPostProcessSteps, and generated Rosetta functions all create mutable state fresh per call and hold only immutable config/factory fields — safe for concurrent invocation.javax.xml.validation.Validatorwas reused across all samples in a test pack.PipelineFunctionRunnerProvider/Implnow take/hold aSchemaand callschema.newValidator()per invocation.TransformTestExtensionupdated to match.ValidationSummariser.addValidationReport(...)is now synchronized since it's a caller-supplied interface.Test plan
mvn test— all 94 tests pass.update-expectationsbuild: therun()phase went from 12.28 min to 4.43 min, and the regenerated expectation files were byte-for-byte identical to the pre-existing committed baseline.