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

Idle Muuri Board crashing browser? #480

Closed
ibrychgo opened this issue Jun 10, 2021 · 9 comments
Closed

Idle Muuri Board crashing browser? #480

ibrychgo opened this issue Jun 10, 2021 · 9 comments
Labels

Comments

@ibrychgo
Copy link

While everything is going well with our board...I've discovered that a board left idle for 20-30 minutes crashes the browser ('Aw Snap!') without errors logged.

When I monitor memory usage, it looks like pe._onWorkerMessage is growing 2 mb every second. Every now and then it goes down a bit but mostly it's a straight path up.

I removed all of our watchers and database connections, so ONLY the muuri board initialization code runs. And, in an idle state, none of our functions are firing. But that webworker keeps growing until crash.

My HTML and javascript for instantiating the boards is nearly identical to the kanban demos. And, since this is an idle test, none of the functions that fire after drag/drop are being called.

Since this is in the webworker...I'm unsure how to proceed investigating. Any ideas?

@ibrychgo
Copy link
Author

ibrychgo commented Jun 10, 2021

So after more testing, disabling different parts of the muuri declaration, I narrowed it down to Muuri's own even listeners.

It seems that layoutStart, if enabled, immediately causes this 1.5 - 2mb memory increase every second.
Even if all other event listeners are disabled.

If layoutStart is not declared, memory seems to grow and decrease as expected. Growing during activity and cleaning up after.

We were only using it because we thought it was necessary as it was included in some of Muuri's kanban examples so, for our project, this is resolved.

The offending listener:

 .on('layoutStart', function () {
          // Let's keep the board grid up to date with the
          // dimensions changes of column grids.
           boardGrid.refreshItems().layout();
     })

But if anyone DOES need to listen for layoutStart...I have a feeling they will eventually run out of memory and crash.

@niklasramo
Copy link
Collaborator

niklasramo commented Jun 11, 2021

@ibrychgo Interesting! Am I getting this right that if I add an empty layoutStart listener memory allocation will keep growing or is the problem in the boardGrid.refreshItems().layout(); part inside that listener? Also, if possible, could you provide a reduced test case (in e.g. codepen/codesandbox) which demonstrates this problem? Will be easier for me to debug it if I can jump straight to the meat of it.

@niklasramo
Copy link
Collaborator

As a temporary fix you can force Muuri to run all layout calculations synchronously in main thread too. Just do this once somewhere, before instantiating any grids:

Muuri.defaultPacker.destroy();
Muuri.defaultPacker = new Muuri.Packer(0);

This code will destroy the default Packer instance and replace it with a new Packer instance that is explicitly told to spawn 0 workers.

@ibrychgo
Copy link
Author

Correct.

An empty layoutStart listener causes memory use to grow.

If it's completely empty, it grows but at a modest amount (around 24k/second.) But that amount does keep increasing (i.e. after 15 seconds it's at 123k/second.) I've not tested this state long enough to see it crash.

If the listener includes boardGrid.refreshItems().layout(); memory increases about 1.5-2mb per second until crash.

I've never made a codepen before but I'm willing to try later today. :) I'll post the link here to let you know.

Thanks for being so engaged and responsive!

@thednp
Copy link

thednp commented Jun 28, 2021

@niklasramo can this fix help with #473 ?

@niklasramo
Copy link
Collaborator

@thednp I don't think this issue is related to #473.

@ibrychgo It turns out that there is a memory leak in the Packer code and just created a fix for it here: #486. Would you mind testing if that PR fixes the memory leak you were having?

@ibrychgo
Copy link
Author

ibrychgo commented Jul 7, 2021 via email

@niklasramo
Copy link
Collaborator

niklasramo commented Jul 7, 2021

@ibrychgo No need to use git for testing this one, here is a direct link to the muuri lib in that PR: https://cdn.jsdelivr.net/gh/haltu/muuri@r0.9.5/dist/muuri.js. If you still can recreate the memory leak in your own code you can then just see if swapping the CDN link fixes it.

@niklasramo
Copy link
Collaborator

This should be now fixed with the latest release: https://github.com/haltu/muuri/releases/tag/0.9.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants