ref: Avoid cloning events to add timestamp
#1339
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 065587a The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
We are always passing in fresh objects to this method, so instead of cloning this into a new object, we can just put the `timestamp` on the given object directly and return it, saving a bit of processing cost.
mydea
force-pushed
the
fn-upstream/avoid-event-cloning
branch
from
76c72e1
to
065587a
Compare
|
This is modifying the underlying object? Are you sure there aren't any typing problems with this, or possible knock on problems? (I can't think of anything over and above it being 'less clean') |
|
Yes, but as far as I can tell we are always passing in a fresh object into this anyhow. We've been running this on our fork at Sentry without problems (as far as I can tell...) for some time now! |
eoghanmurray
added a commit
to eoghanmurray/rrweb
that referenced
this pull request
Apr 9, 2024
… clone per event
eoghanmurray
added a commit
to eoghanmurray/rrweb
that referenced
this pull request
Apr 9, 2024
… clone per event
|
I've gone a bit further with #1441 to avoid that extra function call at all |
|
And thank you for the contribution! (I couldn't make commits directly to this PR) |
|
(actually #1441 needs some more work) |
eoghanmurray
added a commit
to eoghanmurray/rrweb
that referenced
this pull request
Apr 10, 2024
… clone per event
eoghanmurray
added a commit
to eoghanmurray/rrweb
that referenced
this pull request
Apr 10, 2024
… clone per event
eoghanmurray
added a commit
to eoghanmurray/rrweb
that referenced
this pull request
Apr 10, 2024
… clone per event
eoghanmurray
added a commit
to eoghanmurray/rrweb
that referenced
this pull request
Apr 10, 2024
… clone per event
|
#1441 is ready now as a larger change than this one |
eoghanmurray
added a commit
that referenced
this pull request
Apr 19, 2024
…sion (#1441) performance: remove a nested function call and an object clone during event emission - rename `event` to `eventWithoutTime`, but maintain backwards compatibility - `eventWithTime` (with time) could be renamed to `event` in a future version This is an extension of PR #1339 authored by: mydea <[email protected]>
|
#1441 has been merged and I've tagged you in the merge commit, thank you for the PR! |
billyvg
pushed a commit
to getsentry/rrweb
that referenced
this pull request
Apr 19, 2024
…sion (rrweb-io#1441) performance: remove a nested function call and an object clone during event emission - rename `event` to `eventWithoutTime`, but maintain backwards compatibility - `eventWithTime` (with time) could be renamed to `event` in a future version This is an extension of PR rrweb-io#1339 authored by: mydea <[email protected]>
billyvg
pushed a commit
to getsentry/rrweb
that referenced
this pull request
Apr 19, 2024
…sion (rrweb-io#1441) performance: remove a nested function call and an object clone during event emission - rename `event` to `eventWithoutTime`, but maintain backwards compatibility - `eventWithTime` (with time) could be renamed to `event` in a future version This is an extension of PR rrweb-io#1339 authored by: mydea <[email protected]>
billyvg
pushed a commit
to getsentry/rrweb
that referenced
this pull request
Apr 19, 2024
…sion (rrweb-io#1441) performance: remove a nested function call and an object clone during event emission - rename `event` to `eventWithoutTime`, but maintain backwards compatibility - `eventWithTime` (with time) could be renamed to `event` in a future version This is an extension of PR rrweb-io#1339 authored by: mydea <[email protected]>
billyvg
added a commit
to getsentry/rrweb
that referenced
this pull request
Apr 22, 2024
…event emission (#180) performance: remove a nested function call and an object clone during event emission - rename `event` to `eventWithoutTime`, but maintain backwards compatibility - `eventWithTime` (with time) could be renamed to `event` in a future version This is an extension of PR rrweb-io#1339 authored by: mydea <[email protected]>
jaj1014
pushed a commit
to pendo-io/rrweb
that referenced
this pull request
Apr 30, 2024
…sion (rrweb-io#1441) performance: remove a nested function call and an object clone during event emission - rename `event` to `eventWithoutTime`, but maintain backwards compatibility - `eventWithTime` (with time) could be renamed to `event` in a future version This is an extension of PR rrweb-io#1339 authored by: mydea <[email protected]>
jxiwang
pushed a commit
to amplitude/rrweb
that referenced
this pull request
Jul 31, 2024
…sion (rrweb-io#1441) performance: remove a nested function call and an object clone during event emission - rename `event` to `eventWithoutTime`, but maintain backwards compatibility - `eventWithTime` (with time) could be renamed to `event` in a future version This is an extension of PR rrweb-io#1339 authored by: mydea <[email protected]>
jxiwang
pushed a commit
to amplitude/rrweb
that referenced
this pull request
Oct 16, 2024
…sion (rrweb-io#1441) performance: remove a nested function call and an object clone during event emission - rename `event` to `eventWithoutTime`, but maintain backwards compatibility - `eventWithTime` (with time) could be renamed to `event` in a future version This is an extension of PR rrweb-io#1339 authored by: mydea <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We are always passing in fresh objects to this method, so instead of cloning this into a new object, we can just put the
timestampon the given object directly and return it, saving a bit of processing cost.