Skip to content

Conversation

@JoostK
Copy link
Member

@JoostK JoostK commented Aug 16, 2020

While testing compiler performance with ng9-aot-build-times I noticed RxJS's SafeSubscriber to show up as hot function. I opened ReactiveX/rxjs#5646 to mitigate it, however this change cannot be accepted because it breaks a certain usage pattern. As an alternative, this PR avoids the performance penalty by switching over the anonymous observer object to use 3 separate callbacks instead, such that RxJS doesn't have to create calling context objects using slow Object.create.

On a new CLI app, local testing indicates that this saves about 120ms or 2%, whereas the ng9-aot-build-times repo shows approximately 2 second improvement, or ~6%.

@JoostK JoostK added type: bug/fix area: @angular-devkit/core severity4: memory/performance action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Aug 16, 2020
@alan-agius4
Copy link
Collaborator

@JoostK, can you change the commit msg type to fix? perf is not a valid type in the CLI repo

@JoostK JoostK force-pushed the perf/sync-subscribe branch from d37ad35 to 015c6a6 Compare August 16, 2020 11:33
Copy link
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Aug 16, 2020
@alan-agius4 alan-agius4 merged commit 876df75 into angular:master Aug 16, 2020
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: @angular-devkit/core severity4: memory/performance target: patch This PR is targeted for the next patch release type: bug/fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants