arl / statsviz

🚀 Visualise your Go program runtime metrics in real time in the browser
MIT License
3.19k stars 121 forks source link

fix: statsviz.TimeSeries.GetValue is called multiple times per second #121

Closed mzzsfy closed 8 months ago

mzzsfy commented 8 months ago

close https://github.com/arl/statsviz/issues/119

arl commented 8 months ago

Currently, there's a coupling between stats collection and stats emission, which means that if we have multiple clients to emit stats to, we collect stats multiple times. That's what we're trying to solve

I think there might be a simpler solution than what you're proposing in this PR, that doesn't require a new option nor even requires caching.

So, we want to stats collection from stats emission. By doing so, we'll have a single collection goroutine for N clients. To do this, we need to keep track of the currently connected clients. Our single collection goroutine will push somehow the latest collected stats to N currently connected clients. No need for a cache

Aside from that, I think that we should remove websocket and switch to SSE before tacking the current PR, since the code will likely be different by then.

So, I really thank you for trying to solve it, but you see it'd be have been preferable to discuss it before doing the work and sending the PR. Also, somebody, myself included, could even have already been working on it. So I'm saying that since everybody, you included, time and effort is of great but limited value.

Again, I thank you for your excitement into contributing to the project and don't mean to tone it down, I just think we should coordinate, ok?

So to sum it up, the first thing to do should probably switch to SSE, since that would impact the 2 other PR's. Then we should probably tackle the lagging, if we can still reproduce it, see https://github.com/arl/statsviz/pull/120#issuecomment-1879657616.

Finally come back to the current PR.

mzzsfy commented 8 months ago

Yes, your solution may be more suitable, I will put more effort into sse support

mzzsfy commented 8 months ago

Already solved https://github.com/arl/statsviz/pull/122