Skip to content

feat: Optimize set_property if dependent does not need partial change-event processing#844

Open
RobertJacobsonCDC wants to merge 2 commits intomainfrom
RobertJacobsonCDC_841_event_optimization
Open

feat: Optimize set_property if dependent does not need partial change-event processing#844
RobertJacobsonCDC wants to merge 2 commits intomainfrom
RobertJacobsonCDC_841_event_optimization

Conversation

@RobertJacobsonCDC
Copy link
Copy Markdown
Collaborator

This PR makes set_property faster when property-change machinery is not actually needed.

Previously, set_property always created PartialPropertyChangeEvents for the property being written and for every derived dependent. These hold the previous value of their respective properties so that property change events can be emitted and indexes and value change counters updated. This PR adds a cheap per-property check and does not create partial change events for properties that have no PropertyChangeEvent subscribers, no value-change counters, and no indexes to maintain.

To support that, the change adds:

  • a way for Context to check whether a given event type has any registered handlers;
  • a type-erased property-store hook that answers whether a property needs change processing because it has subscribers, counters, or an index;
  • a set_property fast path that skips partial-event creation for unaffected properties.

The benchmarks were also expanded with a steady-state mixed benchmark that keeps one Context alive, flushes callbacks between measured chunks, and exercises the three relevant cases at once: indexed dependent, counted dependent, and handler-backed dependent. That gives us a way to measure both the new best case and the pessimistic path without distorting results with unbounded callback queue growth.

Local Benchmarks

Local Criterion benchmarks in the ixa repo:

set_property/set_property_no_dependents
                        time:   [19.035 ns 19.063 ns 19.096 ns]
                        change: [−42.210% −40.648% −39.368%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe
set_property/set_property_three_dependents
                        time:   [20.509 ns 20.522 ns 20.536 ns]
                        change: [−75.287% −74.613% −74.072%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
  1 (1.00%) low mild
  3 (3.00%) high mild
  6 (6.00%) high severe
set_property/set_property_three_dependents_mixed
                        time:   [45.268 µs 45.355 µs 45.436 µs]
                        change: [−5.4784% −5.0960% −4.7522%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) low mild
  1 (1.00%) high mild
  1 (1.00%) high severe

The improvement in the last case is spurious: that's the "control" in which we actually can't avoid doing the work. The fact that it isn't a regression, though, is good, because that says that the extra checking this optimization requires is negligible.

The result of a few runs of ixa-epi-covid's comparison benchmarks (make bench-compare) on my local machine:

Benchmark Base (mean) Current (mean) Change Base Var Base Std Dev Current Var Current Std Dev
init/population/10k 10.093 ms 8.1505 ms -19.2% faster 0.04953 ms² 0.2226 ms 0.01145 ms² 0.1070 ms
init/population/100k 107.85 ms 89.923 ms -16.6% faster 3.6279 ms² 1.9047 ms 2.1749 ms² 1.4748 ms
execute/population/10k 80.725 ms 82.416 ms +2.1% slower 1.0578 ms² 1.0285 ms 1.4813 ms² 1.2171 ms
execute/population/100k 1.2950 s 1.2058 s -6.9% faster 0.00155 s² 0.0394 s 0.00526 s² 0.0725 s

@github-actions
Copy link
Copy Markdown

Benchmark Results

Hyperfine

Command Mean [ms] Min [ms] Max [ms] Relative
large_sir::baseline 2.9 ± 0.0 2.8 3.0 1.00
large_sir::entities 11.5 ± 0.2 11.3 12.2 4.04 ± 0.09

Criterion

Regressions (slower)
Group Bench Param Change CI Lower CI Upper
indexing with_query_results_single_indexed_property_entities 25.043% 24.069% 26.071%
indexing query_people_count_single_indexed_property_entities 20.912% 20.652% 21.134%
large_dataset bench_filter_unindexed_entity 18.719% 15.028% 22.800%
large_dataset bench_filter_indexed_entity 14.023% 4.561% 25.206%
large_dataset bench_query_population_derived_property_entities 9.605% 8.946% 10.266%
indexing with_query_results_indexed_multi-property_entities 8.663% 8.308% 9.066%
sampling sampling_single_unindexed_entities 5.582% 5.517% 5.643%
sampling sampling_single_unindexed_concrete_plus_derived_entities 5.161% 4.864% 5.503%
examples example-births-deaths 5.095% 4.100% 5.926%
sampling sampling_multiple_known_length_entities 4.880% 4.006% 5.922%
sampling count_and_sampling_single_unindexed_concrete_plus_derived_entiti 4.806% 4.526% 4.984%
sample_entity sample_entity_single_property_unindexed 1000 4.702% 4.335% 5.049%
indexing query_people_count_indexed_multi-property_entities 4.270% 3.668% 4.703%
counts reindex_after_adding_more_entities 3.089% 2.910% 3.251%
indexing query_people_indexed_multi-property_entities 1.764% 1.572% 1.996%
counts multi_property_indexed_entities 1.733% 1.299% 2.401%
counts index_after_adding_entities 1.509% 1.141% 1.950%
large_dataset bench_match_entity 1.502% 1.181% 1.777%
Improvements (faster)
Group Bench Param Change CI Lower CI Upper
counts concrete_plus_derived_unindexed_entities -18.793% -19.109% -18.503%
sample_entity sample_entity_single_property_unindexed 100000 -15.030% -16.559% -13.418%
counts single_property_unindexed_entities -12.164% -13.027% -11.308%
sample_entity sample_entity_single_property_indexed 1000 -8.402% -8.792% -8.087%
indexing query_people_single_indexed_property_entities -6.841% -8.441% -4.686%
sample_entity sample_entity_single_property_indexed 10000 -6.528% -6.868% -6.280%
examples example-basic-infection -6.343% -8.267% -5.055%
sample_entity sample_entity_single_property_indexed 100000 -6.169% -6.624% -5.710%
sample_entity sample_entity_multi_property_indexed 100000 -4.556% -5.271% -3.544%
sample_entity sample_entity_multi_property_indexed 1000 -4.053% -4.419% -3.608%
sample_entity sample_entity_multi_property_indexed 10000 -3.944% -4.377% -3.608%
algorithm_benches algorithm_sampling_multiple_known_length -3.294% -3.892% -2.903%
sampling sampling_single_l_reservoir_entities -2.512% -2.756% -2.315%
sample_entity sample_entity_whole_population 1000 -2.064% -2.866% -1.202%
sampling count_and_sampling_single_known_length_entities -1.922% -2.161% -1.668%
large_dataset bench_query_population_multi_indexed_entities -1.803% -2.387% -1.193%
Unchanged / inconclusive (CI crosses 0%)
Group Bench Param Change CI Lower CI Upper
sample_entity sample_entity_single_property_unindexed 10000 1.623% 0.624% 2.639%
sample_entity sample_entity_whole_population 100000 -1.460% -2.424% -0.362%
algorithm_benches algorithm_sampling_multiple_l_reservoir -1.459% -1.929% -0.891%
sample_entity sample_entity_whole_population 10000 1.167% -0.527% 3.460%
counts multi_property_unindexed_entities 0.858% 0.399% 1.333%
large_dataset bench_query_population_property_entities -0.842% -2.062% 0.273%
sampling sampling_single_known_length_entities -0.786% -1.302% -0.301%
sampling sampling_multiple_l_reservoir_entities -0.530% -0.607% -0.436%
indexing query_people_multiple_individually_indexed_properties_entities 0.506% 0.061% 0.828%
large_dataset bench_query_population_indexed_property_entities -0.498% -0.769% -0.248%
counts single_property_indexed_entities 0.401% -0.210% 0.843%
large_dataset bench_query_population_multi_unindexed_entities 0.352% -0.716% 1.313%
algorithm_benches algorithm_sampling_single_rand_reservoir -0.294% -0.782% 0.080%
indexing query_people_count_multiple_individually_indexed_properties_enti -0.142% -0.355% 0.026%
indexing with_query_results_multiple_individually_indexed_properties_enti 0.129% -0.134% 0.545%
algorithm_benches algorithm_sampling_single_known_length 0.126% -0.148% 0.561%
algorithm_benches algorithm_sampling_single_l_reservoir 0.073% -0.071% 0.289%
sampling sampling_multiple_unindexed_entities -0.024% -0.100% 0.057%

github-actions bot added a commit that referenced this pull request Apr 14, 2026
Copy link
Copy Markdown
Collaborator

@k88hudson-cfa k88hudson-cfa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Looks good to me. Thanks for adding the benchmark

context.set_property(black_box(person), MixedBaseValue(black_box(value)));
}
},
BatchSize::SmallInput,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for SmallInput?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants