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

perf: fast path for simple default construction #639

Conversation

maartenbreddels
Copy link
Contributor

Benchmark results will come in a referring PR

maartenbreddels added a commit to maartenbreddels/ipywidgets that referenced this pull request Nov 19, 2020
Benchmark results for creating a button `widgets.Button()`
------------------------------------------------ benchmark: 1 tests -----------------------------------------------
Name (time in ms)              Min      Max    Mean  StdDev  Median     IQR  Outliers       OPS  Rounds  Iterations
-------------------------------------------------------------------------------------------------------------------
test_performance_create     2.8579  36.1314  3.3029  2.0412  3.0983  0.2583      1;22  302.7645     266           1
-------------------------------------------------------------------------------------------------------------------

After this PR, in combination with ipython/traitlets#639

------------------------------------------------ benchmark: 1 tests -----------------------------------------------
Name (time in ms)              Min      Max    Mean  StdDev  Median     IQR  Outliers       OPS  Rounds  Iterations
-------------------------------------------------------------------------------------------------------------------
test_performance_create     2.0695  62.7899  2.9496  3.6363  2.3238  0.8066      2;27  339.0254     290           1
-------------------------------------------------------------------------------------------------------------------

Which is a 30% performance increase.
Copy link
Contributor

@rmorshea rmorshea left a comment

Choose a reason for hiding this comment

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

Given that _trait_default_generators is a private attribute (and a relatively recently added on at that) could we just change it to use the (class, name) key you're using in _simple_default? Default value creation is already complicated enough as it is, so it'd be nice if we could avoid the adding a separate cache layer if we can just fold that into an existing abstraction.

If that's not possible though I'd definitely suggest a name like _trait_default_generator_cache since that's a bit more descriptive of what the attribute does. Also, we can probably have the trait continue to use obj.trait_defaults, but have that method inspect the cache before tracing the MRO to find the default generator.

@maartenbreddels
Copy link
Contributor Author

Replaced by #777

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.

2 participants