Module memory leaks on every execution of widget #613
Comments
|
Thanks for opening the issue and for the clear, reproducible example. This is certainly a bug and something I want to fix. I've been preparing for an anywidget tutuorial and talk at SciPy, which has been taking some time.
Completely agree. I've been intending to migrate to this type of execution model since introducing the lifecycle hooks in v0.9.0. At that point we need to officially remove support for the prior named export however. So planning for v0.10.0, but we could fix the memory leak in v0.9. |
|
Thanks for the context, and good luck at SciPy. FYI, for now we ended up working around this on our end by pushing our Here's the PR in case anyone finds the approach informative: rerun-io/rerun#6680 Given how large our widget.js bundle is, this also seems to have other benefits, in particular, in Colab notebooks where we were seeing a pretty lengthy load-process on every execution. If every instance of anywidget is isolated it may be tricky to implement caching using any approach other than a hosted asset. |
|
Ok I got some headway with this experimenting in a standalone HTML file. Below is an example of basically what anywidget does behind the scenes: <!doctype html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Memory Leak</title>
</head>
<script type="module">
add.addEventListener("click", async () => {
let esm = `
let buffer = new ArrayBuffer(100_000_000);
export default {
render() {
buffer; // just referencing here leaks memory
},
}
`;
let blob = new Blob([esm], { type: "application/javascript" });
let url = URL.createObjectURL(blob);
let mod = await import(url);
mod.default.render();
URL.revokeObjectURL(url);
items.appendChild(
Object.assign(document.createElement('p'), {
textContent: "Item",
}),
)
});
</script>
<body>
<h1>Memory Leak</h1>
<button id="add">Add</button>
<div id="items"></div>
</body>
</html>Clicking the button will leak memory for every click. I was able to support freeing memory by having the module export a "free" method that sets <!doctype html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Memory Leak</title>
</head>
<script type="module">
window.imports = [];
add.addEventListener("click", async () => {
let esm = `
let buffer = new ArrayBuffer(100_000_000);
export default {
render() {
buffer; // just referencing here leaks memory
},
free() {
buffer = null;
}
}
`;
let blob = new Blob([esm], { type: "application/javascript" });
let url = URL.createObjectURL(blob);
let mod = await import(url);
mod.default.render();
URL.revokeObjectURL(url);
items.appendChild(
Object.assign(document.createElement('p'), {
textContent: "Item",
}),
)
imports.push(mod);
});
remove.addEventListener("click", () => {
imports.forEach(mod => mod.default.free());
imports = [];
items.innerHTML = "";
});
</script>
<body>
<h1>Memory Leak</h1>
<button id="add">Add</button>
<button id="remove">Clear</button>
<div id="items"></div>
</body>
</html>Right now we support cleaning up resources during model initializaiton and rending with by returning functions from the callback hooks: import anywidget
import traitlets
class LeakyWidget(anywidget.AnyWidget):
_esm = """
let buffer = new ArrayBuffer(100000000);
function render({ model, el }) {
let div = document.createElement("div");
div.innerHTML = `Buffer is is ${buffer.byteLength} bytes.`;
el.classList.add("leaky-widget");
el.appendChild(div);
return () => {
// clean up resources created in render
}
}
export default {
render,
initialize() {
return () => {
buffer = null;
}
}
};
"""
_css = """
.leaky-widget button { background-color: #ea580c; }
"""
value = traitlets.Int(0).tag(sync=True)widget = LeakyWidget()
widgetwidget.close() # closes the front-end comm, will trigger the dispose func from initialize and set `buffer = null`.Since we reload the ESM per model instance, this might be a workaround at the moment. Perhaps we could consider a module-level API for "freeing" resource because that seems neccesary if we are going to reload the ESM, but I still think the goal should be to load the ESM once and cache. |
|
Also thank you for sharing the idea CDN work around. I think that's a good idea for others experiencing this kind of issue or who have "large" widgets on the frontned |
|
I was very confused by this behavior when investigating it, because Modules are not collected until their realm is, even if the module and its exports are not reachable anymore. It's hard to dig up any concrete information about this, but here's one thread I found: https://esdiscuss.org/topic/are-es-modules-garbage-collected-if-so-do-they-re-execute-on-next-import For URLs created using <html>
<body>
<h1>Memory Leak</h1>
<script>
async function leak() {
let esm = `
let buffer = new Uint8Array(100_000_000);
export function render(msg) {
console.log("leaked bytes: ", msg, buffer.byteLength);
}
`;
let blob = new Blob([esm], { type: "application/javascript" });
let url = URL.createObjectURL(blob);
const m0 = await import(url);
m0.render("first");
URL.revokeObjectURL(url);
const m1 = await import(url);
m1.render("second");
}
</script>
<button onclick="leak()">Add</button>
</body>
</html>To fix this in |
|
Thanks for the response.
Completely agree. We should load ESM once, upfront for the widget class. The changes I have for this may break packages that depend on
Unfortunately, I think we'll have to accept that anywidget's built-in hot-reloading for development has a memory leak. I can add a section to the documentation explaining this caveat and suggest that you use a separate development server for very large bundles (e.g., Vite with the anywidget vite plugin). The ergonomics of having built-in HMR for prototyping is just too nice, and perhaps the memory leak is an acceptable trade off for the time being since it happens only in development. |
|
Agreed. A documented memory leak to support hot-reloading during development cycles seems well worth it. Also for probably 95% of users developing small JS-only packages the memory leak is not nearly as problematic. Granted, as wasm continues to gain popularity this kind of issue will probably start to show up more often. |
|
It might be helpful to think of this less like a memory leak and more of an issue of documenting / clarifying the widget lifecycle. A basic counter example demonstrates the current behavior as well. class Widget(anywidget.AnyWidget):
_esm = """
if(!window.numModules) window.numModules = 0;
if(!window.numGlobalInit) window.numGlobalInit = 0;
if(!window.numGlobalRender) window.numGlobalRender = 0;
window.numModules += 1;
let numLocalInit = 0;
let numLocalRender = 0;
function initialize() {
numLocalInit += 1;
window.numGlobalInit += 1;
}
function render(view) {{
numLocalRender += 1;
window.numGlobalRender += 1;
const h1 = document.createElement("h1");
h1.innerText = JSON.stringify({ numModules, numLocalInit, numLocalRender, numGlobalInit, numGlobalRender });
view.el.appendChild(h1);
}}
export default { initialize, render }
"""
|
It might need to involve some kind of string hashing to ensure that widget development can still occur directly in a notebook |
I created an issue on the chromium bug tracker ~2 weeks ago (https://issues.chromium.org/issues/350426234), and finally got a response from someone who understands the spec. This behavior is actually spec compliant, and evicting the module from the module map would be a regression.
|
|
Is there a reason this cannot be resolved by reusing the same widget instance on the Python side? |
Thanks for doing that and sharing the response.
Widget ESM is defined on the class but we grab it from the front end from each view of each class instance. I have branch to change the behavior to reuse ESM (i.e., load once) and share it across all instances in the front end. For HMR we will just have documentation that there is a memory leak and point to this issue for explanation. |
Will change it back once manzt/anywidget#628 is merged and manzt/anywidget#613 solved) Currently the tutorial page is 60 MB which is prohibitively large.
It appears that every time we execute our widget, a new copy of the module is instantiated, and subsequently leaked.
You can replicate this with a module that includes a large const such as:
Repeatedly executing
LeakyWidget()in a cell will leak ~100MB per run.You can see this in a browser tool inspector by taking snapshots after each execution of the cell and seeing newly created
ModuleEnvironmentObjects:Ideally, we would have some mechanism to simply cache and re-use the same module across all widget instances, but it would be acceptable for them to at least get freed correctly.
The text was updated successfully, but these errors were encountered: