Repository navigation
fix: finalize a cached instance built during a close at the next close - #726
Merged
Merged
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Benchmark
Details
| Benchmark suite | Current: eefe5ce | Previous: b7a1775 | Ratio |
|---|---|---|---|
benchmarks/test_guard_by_type.py::test_g16_resolve_by_type |
6882481.558534362 iter/sec (stddev: 5.678813475383938e-9) |
6829021.182046608 iter/sec (stddev: 5.484705719035411e-9) |
0.99 |
benchmarks/test_guard_by_type.py::test_g17_resolve_by_type_large_registry |
6784686.284592966 iter/sec (stddev: 6.423747081822658e-9) |
6697985.105419868 iter/sec (stddev: 4.150738470799008e-9) |
0.99 |
benchmarks/test_guard_cold.py::test_g8_cold_first_resolve |
30852.368785451512 iter/sec (stddev: 0.000032928483830973726) |
30757.49282217767 iter/sec (stddev: 0.00003212381064992869) |
1.00 |
benchmarks/test_guard_cold.py::test_g8b_cold_first_resolve_cached |
24728.553239585548 iter/sec (stddev: 0.0001319345998715771) |
24483.236846288204 iter/sec (stddev: 0.00015154400300924624) |
0.99 |
benchmarks/test_guard_concurrency.py::test_g14_concurrent_cached_hit[1] |
848.4788674499208 iter/sec (stddev: 0.00020358785362665122) |
857.6505269844425 iter/sec (stddev: 0.000031478572667331435) |
1.01 |
benchmarks/test_guard_concurrency.py::test_g14_concurrent_cached_hit[2] |
861.4720094375748 iter/sec (stddev: 0.00007661108420709384) |
852.2634799214209 iter/sec (stddev: 0.000029566400203879867) |
0.99 |
benchmarks/test_guard_concurrency.py::test_g14_concurrent_cached_hit[4] |
824.311483616578 iter/sec (stddev: 0.00028075228894478265) |
767.6217950190414 iter/sec (stddev: 0.0002570266758932922) |
0.93 |
benchmarks/test_guard_concurrency.py::test_g15_concurrent_first_resolve[1] |
24176.92675018871 iter/sec (stddev: 0.0000068239367399431945) |
8156.588001364661 iter/sec (stddev: 0.0001306104049620297) |
0.34 |
benchmarks/test_guard_concurrency.py::test_g15_concurrent_first_resolve[2] |
13205.438659813415 iter/sec (stddev: 0.000010498056818339527) |
6062.480530611141 iter/sec (stddev: 0.00018017904286592358) |
0.46 |
benchmarks/test_guard_concurrency.py::test_g15_concurrent_first_resolve[4] |
6045.137530650579 iter/sec (stddev: 0.0000232428049148342) |
3415.1846852251756 iter/sec (stddev: 0.0002481508400191217) |
0.56 |
benchmarks/test_guard_concurrency.py::test_g15b_concurrent_first_resolve_sibling_children[1] |
20028.634270697385 iter/sec (stddev: 0.00000706439853333875) |
19060.870891138744 iter/sec (stddev: 0.000010501867428911193) |
0.95 |
benchmarks/test_guard_concurrency.py::test_g15b_concurrent_first_resolve_sibling_children[2] |
7954.634718195636 iter/sec (stddev: 0.000012954430237881891) |
7946.913031583689 iter/sec (stddev: 0.000014106057259191144) |
1.00 |
benchmarks/test_guard_concurrency.py::test_g15b_concurrent_first_resolve_sibling_children[4] |
3969.7021744200083 iter/sec (stddev: 0.00003366580341022959) |
3882.0758637220474 iter/sec (stddev: 0.000042286664071694886) |
0.98 |
benchmarks/test_guard_concurrency.py::test_g15c_worker_pool_floor_control[1] |
60679.15140311891 iter/sec (stddev: 0.0000016530974622086325) |
57980.00559382983 iter/sec (stddev: 0.0000016583671333003195) |
0.96 |
benchmarks/test_guard_concurrency.py::test_g15c_worker_pool_floor_control[2] |
30666.380446807718 iter/sec (stddev: 0.000007826030009930658) |
26583.077655991503 iter/sec (stddev: 0.000028387436691761624) |
0.87 |
benchmarks/test_guard_concurrency.py::test_g15c_worker_pool_floor_control[4] |
11031.207377591405 iter/sec (stddev: 0.00001995052011402385) |
10858.506065052692 iter/sec (stddev: 0.000019239849675808495) |
0.98 |
benchmarks/test_guard_lifecycle.py::test_g6_build_child_container |
3153204.676797992 iter/sec (stddev: 2.1204107695961346e-8) |
3109200.162117778 iter/sec (stddev: 1.6655170160438705e-8) |
0.99 |
benchmarks/test_guard_lifecycle.py::test_g6b_build_child_container_auto_scope |
3442681.2083284548 iter/sec (stddev: 1.3674530557437456e-8) |
3355865.1714153057 iter/sec (stddev: 4.5860760432145475e-8) |
0.97 |
benchmarks/test_guard_lifecycle.py::test_g7_request_lifecycle_batch |
6220.30075086975 iter/sec (stddev: 0.000005355523991866202) |
5518.268723840956 iter/sec (stddev: 0.000053690988201758585) |
0.89 |
benchmarks/test_guard_lifecycle.py::test_g7c_event_loop_floor_control |
112598.07605265576 iter/sec (stddev: 9.932716295045303e-7) |
111291.94431021478 iter/sec (stddev: 7.415236228873871e-7) |
0.99 |
benchmarks/test_guard_lifecycle.py::test_g7b_request_cycle_sync |
795642.9320026148 iter/sec (stddev: 5.531747995118882e-8) |
811531.4726922724 iter/sec (stddev: 7.030419208342152e-8) |
1.02 |
benchmarks/test_guard_lifecycle.py::test_g13_teardown_at_scale |
125765.95650227071 iter/sec (stddev: 0.0000010439777481947348) |
128880.30989858645 iter/sec (stddev: 7.515728812940163e-7) |
1.02 |
benchmarks/test_guard_lifecycle.py::test_g13b_teardown_at_scale_async_no_finalizers |
1086.3427258839065 iter/sec (stddev: 0.0012314906078289942) |
1106.9222569933033 iter/sec (stddev: 0.0012045400255381931) |
1.02 |
benchmarks/test_guard_resolve.py::test_g1_transient_resolve |
4083822.9156295564 iter/sec (stddev: 1.8265069499612637e-8) |
3927978.9350432907 iter/sec (stddev: 3.170569802722682e-8) |
0.96 |
benchmarks/test_guard_resolve.py::test_g2_cached_resolve |
6744127.011877839 iter/sec (stddev: 1.1599214283324147e-8) |
6651633.973879918 iter/sec (stddev: 4.456018484664371e-9) |
0.99 |
benchmarks/test_guard_resolve.py::test_g3_deep_chain |
1535164.2418488436 iter/sec (stddev: 2.4158987596323282e-8) |
1540870.274285195 iter/sec (stddev: 2.8074047812986162e-8) |
1.00 |
benchmarks/test_guard_resolve.py::test_g4_wide_resolve |
903958.0708113296 iter/sec (stddev: 3.187018603879424e-7) |
866746.707836144 iter/sec (stddev: 3.635108057427795e-7) |
0.96 |
benchmarks/test_guard_resolve.py::test_g5_cross_scope |
3736297.0139374286 iter/sec (stddev: 2.1459848168561528e-8) |
3642649.193107508 iter/sec (stddev: 1.6992111188829588e-8) |
0.97 |
benchmarks/test_guard_resolve.py::test_g9_context_resolve |
2839586.706670974 iter/sec (stddev: 1.1634801499035885e-7) |
2725232.6428932473 iter/sec (stddev: 1.4077741255402713e-7) |
0.96 |
benchmarks/test_guard_resolve.py::test_g12_override_active_resolve |
1548533.7978748723 iter/sec (stddev: 1.97198413446461e-8) |
1524730.519123093 iter/sec (stddev: 2.5341878937965144e-8) |
0.98 |
benchmarks/test_guard_resolve.py::test_g18_alias_hop |
6690258.74908751 iter/sec (stddev: 4.281685465868504e-9) |
6463835.3904325925 iter/sec (stddev: 4.452053658239917e-9) |
0.97 |
benchmarks/test_guard_validate.py::test_g10_validate_deep_chain |
72926.28812543418 iter/sec (stddev: 0.000011126339635657216) |
53458.71373762307 iter/sec (stddev: 0.00025732724667223246) |
0.73 |
benchmarks/test_guard_validate.py::test_g11_validate_wide |
42101.082594108724 iter/sec (stddev: 0.000014641501437904197) |
41550.27511260399 iter/sec (stddev: 0.000015081427680084859) |
0.99 |
This comment was automatically generated by workflow using github-action-benchmark.
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.
Closes #719.
The lifecycle page and both close docstrings promise that a cached instance still being built on another thread when a close starts "is finalized by the next close". It never was.
The builder appends its item to the container's creation order while the close is iterating it. The close then emptied the list (
close_async) or overwrote it with what stayed pending (close_sync), and that dropped the new item. The instance stayed cached but was no longer queued. Every later close skipped it, and so did reopening, because a resolve after reopening is a cache hit and never queues it again. Its finalizer never ran.Each close now takes a copy of the queue, finalizes that copy newest first, and removes only that prefix from the queue:
del creation_order[:n]inclose_async, andcreation_order[:n] = remaininginclose_sync. An item appended during the close stays queued for the next one. Everything else is unchanged:FinalizerErroraggregation;close_sync()staying queued;The docs needed no change, because they now hold. The two docstrings now say a close covers "the items queued when this starts".
Tests, all through the public API, with
threading.Events inside the slow creator and the already-cached instance's finalizer to make the race deterministic:close_sync()with a build finishing mid-close: the next close finalizes it;close_async();close_sync(): it stays pending, the nextclose_sync()reports it, andclose_async()runs it.All three fail on main, and they passed 5 of 5 runs in a row.
Cost: the copy runs only when a container has cached items to close. Guard benchmarks, 2 runs each, main against branch:
A copy-free version would depend on how a reversed list iterator behaves while the list is appended to, which Python does not document, so I kept the copy.