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

improve performance / reduce allocations of FireEvents #2380

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bollhals
Copy link
Contributor

@bollhals bollhals commented Apr 1, 2021

Various improvements around FireEvents

BindingRegistry

  • Change from List to Hashset to store Hookbindings for faster storage
  • reduced closure allocations by removing LINQ

RuntimeBinding*

  • Caching of lazy created objects like Parameter or Type
  • AsArray extension method => Avoid calling ToArray if it is already an array to reduce allocation
  • Throw on null in constructor and avoid check for null in hashcode (less branches equals faster hashcode)

TestExecutionEngine

  • Removed unused fields
  • GetExecuteArguments / ResolveArguments: remove LINQ and shortcut 0 length arguments
  • FireEvents: Shortcut no / one hook invoke path. Reduce / delay allocations where possible

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).
  • Performance improvement
  • Refactoring (so no functional change)
  • Other (docs, build config, etc)

Checklist:

  • I've added tests for my code. (most of the time mandatory)
  • I have added an entry to the changelog. (mandatory)
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

Proof [Before / After]

FireEvents
FireEvents Before
FireEvents After

GetExecuteArguments
GetExecuteArguments Before
GetExecuteArguments After

ResolveArguments
ResolveArguments Before
ResolveArguments After

@SabotageAndi
Copy link
Contributor

You are faster with sending new PRs then we with reviewing them. 😂

@bollhals
Copy link
Contributor Author

bollhals commented Apr 2, 2021

You are faster with sending new PRs then we with reviewing them. 😂

I had a few hours to spare the last few days. 😊 Just take your time 👍🏼

@SabotageAndi
Copy link
Contributor

We will look at them next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
OSS Iteration 2
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

2 participants