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

Common: Optional Param Dict Name to avoid Parameter Re-Addition #3696

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

holgerd77
Copy link
Member

When experimenting along #3694 I realized that (re-)adding the Common parameters with updateParams() comes with a somewhat significant performance cost and often - e.g. for a shared Common in Client - the addition is not necessary since the parameter set is already there.

This PR adds the ability to give the parameter set a name which is then checked for along further additions to void re-adding unnecessarily.

The PR also removes the resetParam() method. This was a hypothetical method I added, because I thought it might be useful for someone. Seems overblown to me now (respectively we can always re-add), we ourselves use this method nowhere in the monorepo.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 2026979 Previous: 9856f66 Ratio
Block 9422905 6759 ops/sec (±1.29%) 37877 ops/sec (±1.73%) 5.60
Block 9422906 6680 ops/sec (±0.48%) 35937 ops/sec (±3.35%) 5.38
Block 9422907 6415 ops/sec (±3.22%) 36656 ops/sec (±1.46%) 5.71
Block 9422908 6401 ops/sec (±3.82%) 35733 ops/sec (±1.60%) 5.58
Block 9422910 6024 ops/sec (±6.14%) 34971 ops/sec (±1.78%) 5.81

This comment was automatically generated by workflow using github-action-benchmark.

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

Successfully merging this pull request may close these issues.

1 participant