Repository navigation
Run the slow AlgorithmHistoryTests cases in parallel - #9977
Conversation
- Run the five slowest AlgorithmHistoryTests methods in parallel, with a fixture instance per test case so they don't share the algorithm field - Initialize Python.NET's decimal type and PandasData before tests run in parallel: both import Python modules lazily while holding a lock, which deadlocks with a test that holds the GIL and waits on that lock - Move the StreamReaderExtensions performance test to its own non-parallelizable fixture: it compares two timings, and the method-level ParallelScope.None didn't keep it from running next to other tests Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The failing checks other than |
|
I checked the six failing |
There was a problem hiding this comment.
Approving: only test code changes, and I found no defects.
What I checked
AlgorithmHistoryTestslife cycle.FixtureLifeCycle(LifeCycle.InstancePerTestCase)needsOneTimeSetUpto be static, and the providers it sets are now static too. They're read-only after setup, so sharing them is safe. Of the five parallel methods, onlyTickHistoryReturnsConsistentResultsWithOrWithoutEquityuses the_algorithmfield, and each test now gets its own instance. The other four build their own algorithm withGetAlgorithm. None of the five touchConfig,SymbolCache, the custom data statics or_testHistoryProvider. Each case runsPyModule.FromString("testModule", ...), which creates a separate module object, so cases running at the same time don't share one.- State the rest of the fixture changes while these cases run.
TickResolutionOpenInterestHistoryRequestIsFilteredByDefault_MultipleSymbols(Python)callsSymbolCache.Clear(), and the warning tests callConfig.Set("history-request-cells-warning-threshold", ...). The parallel cases assert only bar counts and prices, neverDebugMessagesor symbol strings, so neither change can make them fail. Before this PR, other fixtures already ran alongside thatSymbolCache.Clear(). - Python initialization in
AssemblyInitialize. In the checked-out pythonnet,Runtime.PyDecimalTypeis aninternal static Lazy<PyType>field (src/runtime/Runtime.cs:482), so the reflection reaches the right member.PandasData's static constructor takes the GIL itself, and taking it again inside the outerPy.GIL()is fine. QuantConnect/pythonnet#153 is open and public. - Moved
Performancetest. I compared it with the removed code: it's identical apart from the new fixture and itsusings, andTradeBarTest/QuoteBarTestmoved with it.StreamReaderExtensionsTestshas no unusedusings left. NUnit runs a[NonParallelizable]fixture in the non-parallel work shift, so nothing else runs during it.
CI. Build & Test Lean passed: 39,178 passed, 0 failed. The six red checks are the other workflows, which were cancelled as #9966 asks. I confirmed each run's conclusion is cancelled and no step failed. Saying they tell us nothing about this change is wrong for three of them, though. Regression Tests, Research Regression Tests and Python Virtual Environments all run QuantConnect.Tests.dll, so they run the changed AssemblyInitialize. RegressionTests and ResearchRegressionTests also call AssemblyInitialize.AdjustCurrentDirectory() from their test case sources. So I ran those three workflows' commands myself on this PR's head commit, in the workflow image, and all passed:
- Regression Tests: 1,101 passed, 0 failed
- Research Regression Tests: 5 passed, 0 failed
- Python Virtual Environments:
AssertVirtualEnvironmentpassed with and without system site packages, andPythonPackagesTestspassed 138 of 138
Report Generator Tests, Syntax Tests and Benchmarks don't load the test assembly's setup.
Non-blocking note. The reflection in AssemblyInitialize is all null-conditional. If a later pythonnet upgrade renames or removes PyDecimalType before #153 ships, the warm-up silently does nothing and the parallel Python deadlock can come back. Since #153 is meant to make that line removable, I'm fine leaving it as is.
Other repositories: none affected. The change is limited to the test assembly.
Risk: I set risk:low. The change is test-only and easy to revert. The evidence is the green main workflow, 32 stress runs of the changed fixtures with no hangs, and my passing runs of the three affected workflows that were cancelled.
Part of #9966 (stage 2 of the plan posted on the issue).
Description
AlgorithmHistoryTestsran its 345 tests one after another (164s of test time). It was the last fixture still running in the parallel phase, alone for ~150s. Five methods account for ~140s of that:HistoryRequestWithExtendedMarketHoursTickResolution,TickHistoryReturnsConsistentResultsWithOrWithoutEquity,HistoryRequestWithContracDepthOffsets,HistoryRequestWithDataMappingModeandHistoryRequestWithFillForward.AlgorithmHistoryTests: those five methods are now[Parallelizable(ParallelScope.All)], so their cases run in parallel with each other and with the rest of the fixture. The rest of the fixture still runs one test at a time, as before (checked in the results: no two of its other tests overlap). The fixture now usesFixtureLifeCycle(LifeCycle.InstancePerTestCase), so each test gets its own_algorithmand_testHistoryProvider. ItsOneTimeSetUpand the providers it sets are now static, as that life cycle requires. No test or assertion changes.I didn't make the whole fixture parallel. Many of its other tests change process-wide state, such as
Config(the history warning tests),SymbolCache.Clear(), custom data registered under the same ticker in its C# and Python cases, and the static counters ofCustomFundamentalTestData. One attempt failedGetHistoryWithCustomDataType(Python)this way. The five methods above only read the standard test data through their own algorithm instance.AssemblyInitialize: running Python tests at the same time deadlocked the test host twice. Both times it was a lazily initialized value that imports a Python module while it holds a lock: Python.NET'sRuntime.PyDecimalType(aLazythat imports_pydecimal), and thePandasDatastatic constructor (which importsPandasMapper). During the import, the interpreter hands the GIL to another thread. If that thread needs the same value, it waits on the lock while holding the GIL, and the importing thread never gets the GIL back. Stack dumps of the stuck host showed exactly that. Both values are now initialized once in the assembly setup, before any test runs. For Python.NET's value that takes reflection, since it's internal. I reported the root cause in Deadlock when two threads first resolve PyDecimalType at the same time pythonnet#153. Once that ships, the reflection line can go. ThePandasDatastatic constructor is library code: the same deadlock can happen in any process where two threads first usePandasDataat the same time while one holds the GIL. This PR doesn't change it.StreamReaderExtensionsTests.Performance: it compares two readers' timings (streamReaderMs * 1.5 < getLineReaderMs). Its method-level[Parallelizable(ParallelScope.None)]didn't keep it from running next to other tests. In the results it ran at the very start of the parallel phase, onmastertoo. It already fails now and then on CI (for example run 39981629984). In my first full run of this branch it failed while overlapping with the slow history cases. It now lives in its own[NonParallelizable]fixture,StreamReaderExtensionsPerformanceTests, which runs with nothing else in the same test run. The test and its assertion are unchanged.Only test code changes. The workflow file is not changed.
Testing
Measured in
quantconnect/lean:foundation(--cpus 12 --memory 12g) with the workflow's build and test commands. Eachmaster(a3bf4cc) / branch pair ran at the same time on the same host, which was shared and loaded.masterAlgorithmHistoryTestsaloneAlgorithmHistoryTestsinside the full run (first start to last end)dotnet testduration)Run 1 failures: on
master,LiveTradingDataFeedTests.HandlesAllTypes<TradeBar>(SPY ... Minute ...), the known fixed-wait test that stage 3 covers. On the branch,StreamReaderExtensionsTests.Performance(QuoteBar), which led to the last change above. Run 2 includes that change, and therePerformanceran with no other test overlapping it.Consistency: the changed fixtures (
AlgorithmHistoryTestsand bothStreamReaderExtensionsfixtures, 387 tests) ran 32 times, 8 in a row in each of 4 containers running at the same time (3 CPUs each).AlgorithmHistoryTestspassed all 32 runs, with no hangs (2m 9s – 2m 34s per run, against 3m 1s – 3m 11s onmasterin the same setup).Performancefailed in 5 of the 32 runs. The same stress onmasterfailed it in 3 of 32, with the same assertion. In this setup the noise comes from the other containers on the host, which no test attribute can isolate. That is also true on shared CI runners.What's left for the next stages
LiveTradingDataFeedTests: wait on conditions instead of fixed time spans.RemoteFileSubscriptionStreamReaderTests,LiveTradingResultHandlerTests,LiveTradingRealTimeHandlerTests,KeyStringSynchronizerTests,RateGateTests, ...).BrokerageTransactionHandlerTests,CustomLiveDataFeedTests, ...). WithAlgorithmHistoryTestsat ~120s, it's still among the longest fixtures in the parallel phase. Most of what's left in its parallel cases is waiting for the GIL, which every history call takes for each slice once Python is initialized.🤖 Generated with Claude Code