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 leaking memory due to not improper cleanup of references to detached nodes #1645

Merged
merged 3 commits into from
Dec 10, 2024

Conversation

Kuuuube
Copy link
Member

@Kuuuube Kuuuube commented Dec 10, 2024

Might fix #922, #836, #1480

I can't say for certain this is all of the issue but I wasn't able to find anything else in my testing.

@Kuuuube Kuuuube added kind/bug The issue or PR is regarding a bug area/performance The issue or PR is related to performance labels Dec 10, 2024
@Kuuuube Kuuuube requested a review from a team as a code owner December 10, 2024 05:20
@djahandarie
Copy link
Collaborator

Nice! Just curious, how did you discover these? It'd be nice if we had some visualization for the size of "state" like this over time faceted by variable name but seems like a hard problem in the abstract

@Kuuuube
Copy link
Member Author

Kuuuube commented Dec 10, 2024

Nice! Just curious, how did you discover these? It'd be nice if we had some visualization for the size of "state" like this over time faceted by variable name but seems like a hard problem in the abstract

To see the detached elements: pull up the search page > F12 > Memory > Profiles > Detached elements > Start

To figure out what is causing them I just checked anywhere that could reference any of the detached elements it found and commented stuff out until there were no detached elements. Went back and uncommented stuff until it broke again. Then drilled down through whatever methods were being called commenting stuff out and testing until I finally got whatever sorted out.

This took hours.

@jamesmaa jamesmaa added this pull request to the merge queue Dec 10, 2024
Merged via the queue into yomidevs:master with commit e38c1f0 Dec 10, 2024
11 checks passed
@AdriGrana
Copy link

Wow that's some really good news! I shall try it and report back. Thanks a lot for the good work as always.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/performance The issue or PR is related to performance kind/bug The issue or PR is regarding a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Yomitan crashes on Google Chrome multiple times an hour
4 participants