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

Babel.transform is leaking with goja #250

Closed
mstoykov opened this issue Jan 22, 2021 · 1 comment
Closed

Babel.transform is leaking with goja #250

mstoykov opened this issue Jan 22, 2021 · 1 comment
Labels

Comments

@mstoykov
Copy link
Contributor

I don't know if this is a babel issue actually, but it exists both with the very old babel we use in k6 as well as the latest one, and I can't find any issues about it so 🤷 .

The following test code

package goja

import (
        "io/ioutil"
        "net/http"
        "testing"
)

func TestBabelLeaks(t *testing.T) {
        // r, _ := http.Get("https://unpkg.com/@babel/standalone/babel.min.js") // work with presets
        // "env" instead of "latest" for example
        r, _ := http.Get("https://raw.githubusercontent.com/loadimpact/k6/master/js/compiler/lib/babel.min.js")
        b, _ := ioutil.ReadAll(r.Body)
        _ = r.Body.Close()

        s := MustCompile("babel.min.js", string(b), false)

        vm := New()
        _, err := vm.RunProgram(s)
        if err != nil {
                panic(err)
        }
        for i := 0; i < 200; i++ {
                _, err := vm.RunString(`Babel.transform("let a = 41; a++;", {"presets": ["latest"]})`)
                if err != nil {
                        panic(err)
                }
        }
}

will take (roughly) twice as much memory if the 200 becomes a 400. This seems to indicate that it's leaking. This is especially obvious with the running of the tc39 tests in k6 as they take upwards of 4GB now and enabling almost all tests (most of which obviously fail) goes up to 24GB+. But if I change the code so we constantly re run (or even recompile) babel it works without going over like ... 100mb or something like that. Obviously though that makes the running time from under 20 minutes to nearly 2 hours 😭 .

Looking at pprof output didn't help much except to show that stash.CreateBinding is top2 and that stash.DeleteBinding has never been called in the git history of the project. But I am not certain that matters 🤷

I would expect babel does something "normal" that ends up not being very well supported by goja. Maybe Weak* collections 🤔 , but AFAIK the k6 one works without Weak* collections, so 🤷‍♂️

@dop251
Copy link
Owner

dop251 commented Jan 23, 2021

This is to do with WeakMaps. From the current README:

WeakMap maintains "hard" references to its values. This means if a value references a key in a WeakMap or a WeakMap itself, it will not be garbage-collected until the WeakMap becomes unreferenced.

It appears that Babel is doing exactly that 😞 Good news is there is a much simpler approach to implementing WeakMaps which is also not without caveats but would work in this case. I'm going to push a change shortly.

@dop251 dop251 added the bug label Jan 23, 2021
@dop251 dop251 closed this as completed in eb3de9e Jan 23, 2021
tsedgwick pushed a commit to mongodb-forks/goja that referenced this issue Apr 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants