search

livebud / bud

The Full-Stack Web Framework for Go
MIT License
5.53k stars 182 forks source link

Memory Profiling a Bud service #372

Open Fuerback opened 1 year ago

Fuerback commented 1 year ago

I'm wondering if there is a way to do a Memory Profiling in a Bud service because I'm noticing a possible memory leak.

I'm reading about the native Golang package pprof, but to do this analysis is necessary to have access to the main package or routers.

The image below shows the service is using more and more memory, it doesn't stop. Screenshot from 2023-02-01 09-04-21

I was using the 0.2.5 version and recently updated it to 0.2.8, but it happens in both versions.

matthewmueller commented 1 year ago

Hey @Fuerback, thanks for doing this research! I'd say it's still a bit early days for memory optimization in Bud, but I did see https://github.com/rogchap/v8go/issues/367 the other day, which is potentially related.

Are you experiencing out of memory (OOM) errors? It might just build up and then garbage collection triggers after a bit.

If you dig any deeper, please share your findings!

Fuerback commented 1 year ago

Hey @matthewmueller, I'm not experiencing any OOM errors.

I'll share if I find something interesting.

Thanks for the quick answer!

dobarx commented 1 year ago

V8 isolate is never closed: https://github.com/livebud/bud/blob/bff7b1fac6e69bacdabf524376ee564c6fdd2cb9/package/js/v8/v8.go#L17

dobarx commented 1 year ago

Also, if reusing isolates, value should be released after Eval. This could also leak memory. New v8go release has:

defer value.Release()

I've had a similar problem on experiment side project where I tried to write simpler svelte renderer in Go. I wanted to reuse isolates for better performance. But memory usage would just spike up since rendered html would not get cleared from memory.

Now I tried adding value.Release() and it seems it solved this issue.

matthewmueller commented 1 year ago

@dobarx, thanks for your research!

Right now we are re-using the isolates and context for the duration of the running process, but they do get cleaned up on close:

https://github.com/livebud/bud/blob/bff7b1fac6e69bacdabf524376ee564c6fdd2cb9/package/js/v8/v8.go#L133-L138

This feels like the right approach so only need to load these svelte templates once, then when rendering HTML, it's just calling those functions over and over again. I haven't had the time to do any more in-depth research on this topic yet.

FWIW, my thinking around keeping context around is that since we're running developer-controlled scripts, we're not concerned about security issues that would otherwise crop up from user-run scripts. It's a bit faster too IIRC.

You are right that we aren't currently releasing the value:

https://github.com/livebud/bud/blob/bff7b1fac6e69bacdabf524376ee564c6fdd2cb9/package/js/v8/v8.go#L113-L131

value.Release(), does seem promising.