Skip to content

Conversation

vsariola
Copy link
Contributor

@vsariola vsariola commented Jun 25, 2025

Line 475 of d3d11_windows.go, where the driver allocates a new Buffer object, caused > 20% of all object allocations in Sointu:

image

image

I switched to using a simple, global sync.Pool for the buffers; the buffer is returned to the Pool during Release. This made the garbage disappear:

image

sync.Pool is thread safe and performant even when accessed from multiple threads, so static global pool should be fine. The only gotcha here is that the buffer object should NOT be used after calling Release, as it is returned to the Pool, but that seems to be the case already.

Copy link
Member

@whereswaldon whereswaldon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eliasnaur This looks like a straightforward win to me. Are there any subtleties to worry about with the lifecycle of these buffers?

@whereswaldon whereswaldon requested a review from eliasnaur June 25, 2025 14:27
@eliasnaur
Copy link
Contributor

I don't like complexity in the backends, especially when the renderer really shouldn't allocate so many buffers.

Fixing the renderer is probably too ambitious, but I'd accept a similar change that pools buffers in the renderer. That way, we also speed up the other backends, and even rendering itself (if GPU buffer allocation is a bottleneck).

@vsariola
Copy link
Contributor Author

Uh oh, I tried looking into the higher level and it's a bit above my paygrade. I'm wondering if all these objects are getting created unnecessarily in the first place, because the drawOps.pathCache ain't actually caching anything?

image

Could this be a bug in the pathCache or is it just that my layout is actually changing so much that it cannot be cached?

@vsariola
Copy link
Contributor Author

I tried and failed so far. But the approach I took was using the hash/fnv (https://pkg.go.dev/hash/fnv) to compute a hash from the actual op data and use that as a key to a cache, instead of using opKey / Key that it is using currently. This way we would not need to refresh the cache when version changes, as long as the data is the same, the path should be the same. Am I at all in the right track here?

@eliasnaur
Copy link
Contributor

I'm not sure, sorry. It's been years since I touched the renderer. Does the hashing produce garbage? If so, that would defeat its purpose.

@vsariola
Copy link
Contributor Author

The Hash interface has Reset() function so that same hashing function can be used several times. They are interfaces so the hash state is located in heap, but we only need one. https://pkg.go.dev/hash#Hash. So, no garbage beyond the initial heap allocation for the hash state (https://cs.opensource.google/go/go/+/refs/tags/go1.24.4:src/hash/fnv/fnv.go;l=65)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants