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

fix: Performance issue with player logs #17948

Merged
merged 7 commits into from
Oct 12, 2023
Merged

Conversation

benjackwhite
Copy link
Contributor

@benjackwhite benjackwhite commented Oct 12, 2023

Problem

Long bit of debugging led me to find out something frustrating - the logging from the replayer is the source of a huge number of lock ups. Essentially the logged object can be so massive and full of getter proxies that simply serializing it takes such a long time that the browser locks up.

Any recording with a lot of warnings suffered from this which is largely why some people had such bad experiences.

Changes

  • Fixes it so we don't log, but rather just count and delay a message indicating to the viewer how they could get to the error messages without actually logging them to the console
  • Also accidentally includes some Notebook tidying - not too much there and I've checked it works so just going to lazily include it 🙈

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

How did you test this code?

Copy link
Member

@pauldambra pauldambra left a comment

Choose a reason for hiding this comment

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

additional files but 🚀

Copy link
Member

Choose a reason for hiding this comment

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

git checkout origin/master -- frontend/src/scenes/notebooks/NotebookSelectButton/NotebookSelectButton.tsx is what I use to remove files from PRs when they sneak in

Copy link
Member

Choose a reason for hiding this comment

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

git checkout origin/master -- frontend/src/scenes/notebooks/Notebook/SlashCommands.tsx is what I use to remove files from PRs when they sneak in

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh sweet. I will endeavour to do that next time 🙈

Copy link
Member

Choose a reason for hiding this comment

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

I will never forgive you (it's sad, but we have to maintain standards)

@benjackwhite benjackwhite enabled auto-merge (squash) October 12, 2023 14:57
@benjackwhite benjackwhite disabled auto-merge October 12, 2023 15:03
@benjackwhite benjackwhite merged commit 36521eb into master Oct 12, 2023
@benjackwhite benjackwhite deleted the feat/notebook-improv branch October 12, 2023 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants