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(elements): improve performance of real-time reporting #2498

Merged
merged 5 commits into from May 11, 2023

Conversation

xandervedder
Copy link
Contributor

@xandervedder xandervedder commented May 8, 2023

Instead of recalculating every time an event comes through, only recalculate it during certain intervals. By using the debounceTime function from rxjs we can tell rxjs to ignore certain events during a certain time period.

before: https://profiler.firefox.com/public/m0578rpmtf3xxvq31wfwd5927jbsdgmtcgs3ekr/calltree/?globalTrackOrder=0w2&implementation=js&thread=2&v=9

after: https://profiler.firefox.com/public/fftmqbk76rwn26g2p9wwjzwwr6ef7b6gqh052g8/calltree/?globalTrackOrder=0w3&implementation=js&thread=5&v=9

closes #2496

@xandervedder xandervedder added the enhancement New feature or request label May 8, 2023
@xandervedder xandervedder self-assigned this May 8, 2023
@nicojs
Copy link
Member

nicojs commented May 8, 2023

Why don't we implement fine-grained reactivity instead? Might be fast enough to handle big workloads without the need for throttling

@xandervedder
Copy link
Contributor Author

Right now, the function that costs the most performance is recalculating the metrics since that is done recursively for every item in the model (which could be a large model).

I remember you told me we could mark the mutants (?) as dirty so we would only have to update the marked ones. Not sure if that would improve performance and also not sure when we would update the marked ones.

This might not be the best solution, but it definitely improves performance 😃.

@rouke-broersma
Copy link
Member

Why don't we implement fine-grained reactivity instead? Might be fast enough to handle big workloads without the need for throttling

That sounds like a good future improvement, but at this stage I think that might be a bit too much out of scope considering the other work that still needs to be done (real time report support in the dashboard for example). Could you accept this throttling solution in the intermediate time?

@hugo-vrijswijk
Copy link
Member

FYI the conventional commit type should be perf: https://github.com/angular/angular/blob/main/CONTRIBUTING.md#type

@xandervedder xandervedder changed the title fix(elements): improve performance perf(elements): improve performance May 9, 2023
Instead of recalculating everytime an event comes through, only event it
during certain intervals. By using the `throttleTime` function from rxjs
we can tell rxjs to ignore certain events during a certain time period.
Unfortunately I have to use the real SSE object, since otherwise the
logic that would dispatch the event, would be gone. I still want to test
more, the new listener for example.
@xandervedder xandervedder marked this pull request as ready for review May 10, 2023 07:00
@xandervedder
Copy link
Contributor Author

xandervedder commented May 10, 2023

There seems to be a test on windows that is timing out, investigating that right now...

I am not able to reproduce it locally...

@rouke-broersma rouke-broersma changed the title perf(elements): improve performance perf(elements): improve performance of real-time reporting May 11, 2023
@rouke-broersma rouke-broersma merged commit 42f8dcf into master May 11, 2023
11 checks passed
@rouke-broersma rouke-broersma deleted the feat/2496/performance branch May 11, 2023 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

realtime reporting: improve performance when recieving large amounts of mutant events
4 participants