Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor optimizations in hot codepaths accessing class parameters #893

Merged
merged 5 commits into from Jan 11, 2024

Conversation

philippjfr
Copy link
Member

No reason why the have to call .objects to get the list of parameters and also no reason we have check all parameters.

@philippjfr
Copy link
Member Author

Actually this probably does not take into account dynamically added parameters added using .add_parameter

@philippjfr
Copy link
Member Author

Should be fixed now.

@philippjfr philippjfr changed the title Optimize .params.__contains__ Minor optimizations in hot codepaths accessing class parameters Dec 17, 2023
@philippjfr
Copy link
Member Author

Not showing a huge impact on the benchmarks but those don't cover real usecases yet, which end up calling __contains__ and a few other similar methods a whole lot.

       before           after         ratio
     [a654e784]       [e47cf4bd]
     <main>           <optimize_contains>
       41.9±0.5ms       41.1±0.3ms     0.98  benchmarks.ImportSuite.timeraw_import_param
      16.2±0.03μs      16.2±0.08μs     1.00  benchmarks.ParameterSuite.time_instantiation
      11.8±0.09μs      10.9±0.06μs     0.92  benchmarks.ParameterizedDependsInstantiateSuite.time_declarative_10_parameters_separate_cb
      11.8±0.08μs      10.8±0.02μs     0.92  benchmarks.ParameterizedDependsInstantiateSuite.time_declarative_10_parameters_shared_cb
      9.66±0.07μs      8.89±0.05μs     0.92  benchmarks.ParameterizedDependsInstantiateSuite.time_declarative_1_parameter
          423±2μs          412±6μs     0.97  benchmarks.ParameterizedDependsInstantiateSuite.time_watch_10_parameters_separate_cb
          369±4μs          360±2μs     0.98  benchmarks.ParameterizedDependsInstantiateSuite.time_watch_10_parameters_shared_cb
       51.4±0.1μs       50.6±0.2μs     0.99  benchmarks.ParameterizedDependsInstantiateSuite.time_watch_1_parameter
          679±4μs          670±2μs     0.99  benchmarks.ParameterizedDependsSuite.time_declarative_10_parameters_separate_cb
          634±1μs          630±4μs     1.00  benchmarks.ParameterizedDependsSuite.time_declarative_10_parameters_shared_cb
          109±1μs        108±0.5μs     0.99  benchmarks.ParameterizedDependsSuite.time_declarative_1_parameter
          677±4μs          669±3μs     0.99  benchmarks.ParameterizedDependsSuite.time_watch_10_parameters_separate_cb
          640±4μs          626±1μs     0.98  benchmarks.ParameterizedDependsSuite.time_watch_10_parameters_shared_cb
          111±2μs        107±0.2μs     0.97  benchmarks.ParameterizedDependsSuite.time_watch_1_parameter
       31.9±0.3μs       30.1±0.1μs     0.94  benchmarks.ParameterizedInstantiateSuite.time_100_parameters
       11.8±0.1μs      10.8±0.06μs     0.92  benchmarks.ParameterizedInstantiateSuite.time_10_parameters
      9.79±0.08μs       9.06±0.3μs     0.93  benchmarks.ParameterizedInstantiateSuite.time_1_parameters
      3.10±0.02μs         3.08±0μs     1.00  benchmarks.ParameterizedSetattrSuite.time_class
      2.87±0.02μs      2.88±0.01μs     1.00  benchmarks.ParameterizedSetattrSuite.time_instance
       35.0±0.6μs       35.0±0.3μs     1.00  benchmarks.ParameterizedSuite.time_class_bare
      5.46±0.01ms      5.46±0.01ms     1.00  benchmarks.ParameterizedSuite.time_class_with_100_parameter
          583±3μs         596±20μs     1.02  benchmarks.ParameterizedSuite.time_class_with_10_parameter
       92.4±0.5μs         95.4±5μs     1.03  benchmarks.ParameterizedSuite.time_class_with_1_parameter
       12.5±0.2μs      12.4±0.04μs     0.99  benchmarks.WatcherSuite.time_trigger

Copy link
Member

@jbednar jbednar left a comment

Choose a reason for hiding this comment

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

Looks good so far!

@philippjfr
Copy link
Member Author

I'm done here for now, those py3.12 test failures seem unrelated.

@Hoxbro
Copy link
Member

Hoxbro commented Dec 18, 2023

The failing test is because of pytest-asyncio. The next release (0.23.3) should fix it.

pytest-dev/pytest-asyncio#711

@maximlt maximlt self-requested a review December 18, 2023 15:37
@maximlt
Copy link
Member

maximlt commented Dec 22, 2023

I added two simple benchmarks for x in <inst|class>.param(ParameterizedParamContainsSuite) which indeed show some significant speed-up. @philippjfr don't hesitate to add more "real-world" benchmarks if you have some in mind.

       before           after         ratio
     [a654e784]       [e47cf4bd]
     <main>           <optimize_contains>
       34.7±0.1ms       34.9±0.6ms     1.01  benchmarks.ImportSuite.timeraw_import_param
       16.8±0.1μs       16.5±0.1μs     0.98  benchmarks.ParameterSuite.time_instantiation
      8.81±0.05μs       8.16±0.1μs     0.93  benchmarks.ParameterizedDependsInstantiateSuite.time_declarative_10_parameters_separate_cb
      8.81±0.09μs      8.08±0.03μs     0.92  benchmarks.ParameterizedDependsInstantiateSuite.time_declarative_10_parameters_shared_cb
      7.03±0.03μs      6.51±0.02μs     0.93  benchmarks.ParameterizedDependsInstantiateSuite.time_declarative_1_parameter
          351±4μs          337±5μs     0.96  benchmarks.ParameterizedDependsInstantiateSuite.time_watch_10_parameters_separate_cb
        313±0.5μs          314±8μs     1.00  benchmarks.ParameterizedDependsInstantiateSuite.time_watch_10_parameters_shared_cb
       41.1±0.1μs       40.2±0.3μs     0.98  benchmarks.ParameterizedDependsInstantiateSuite.time_watch_1_parameter
          562±3μs         559±10μs     0.99  benchmarks.ParameterizedDependsSuite.time_declarative_10_parameters_separate_cb
        530±0.7μs          530±8μs     1.00  benchmarks.ParameterizedDependsSuite.time_declarative_10_parameters_shared_cb
       92.2±0.4μs       90.8±0.4μs     0.98  benchmarks.ParameterizedDependsSuite.time_declarative_1_parameter
          569±3μs         555±10μs     0.97  benchmarks.ParameterizedDependsSuite.time_watch_10_parameters_separate_cb
          530±3μs          527±2μs     1.00  benchmarks.ParameterizedDependsSuite.time_watch_10_parameters_shared_cb
       91.7±0.8μs         91.2±2μs     0.99  benchmarks.ParameterizedDependsSuite.time_watch_1_parameter
       25.7±0.1μs      24.5±0.07μs     0.95  benchmarks.ParameterizedInstantiateSuite.time_100_parameters
       8.93±0.2μs       8.30±0.2μs     0.93  benchmarks.ParameterizedInstantiateSuite.time_10_parameters
      7.06±0.09μs       6.48±0.1μs     0.92  benchmarks.ParameterizedInstantiateSuite.time_1_parameters
       65.9±0.6ns       67.1±0.7ns     1.02  benchmarks.ParameterizedParamAccessSuite.time_class
          175±1ns          177±1ns     1.01  benchmarks.ParameterizedParamAccessSuite.time_instance
-         502±2ns          174±2ns     0.35  benchmarks.ParameterizedParamContainsSuite.time_class
-         641±7ns          284±1ns     0.44  benchmarks.ParameterizedParamContainsSuite.time_instance
      2.61±0.02μs      2.61±0.01μs     1.00  benchmarks.ParameterizedSetattrSuite.time_class
      2.31±0.01μs      2.31±0.01μs     1.00  benchmarks.ParameterizedSetattrSuite.time_instance
       31.2±0.2μs       31.2±0.1μs     1.00  benchmarks.ParameterizedSuite.time_class_bare
      4.55±0.02ms      4.58±0.01ms     1.01  benchmarks.ParameterizedSuite.time_class_with_100_parameter
          492±4μs          490±3μs     0.99  benchmarks.ParameterizedSuite.time_class_with_10_parameter
       79.2±0.2μs       79.5±0.4μs     1.00  benchmarks.ParameterizedSuite.time_class_with_1_parameter
      8.63±0.09μs      8.61±0.07μs     1.00  benchmarks.WatcherSuite.time_trigger

@philippjfr philippjfr merged commit 4d2f23c into main Jan 11, 2024
10 checks passed
@philippjfr philippjfr deleted the optimize_contains branch January 11, 2024 13:08
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.

None yet

4 participants