logicomacorp / WaveSabre

Official WaveSabre repository
MIT License
246 stars 34 forks source link

WIP: Array malloc/free #41

Closed kusma closed 3 years ago

kusma commented 4 years ago

Here's some old patches I had laying around to avoid using the array versions of new/delete instead of malloc/free that I wrote while trying to solve some CRT issues that I was able to work around in another way in the end.

But I recently realized that these save us 32 bytes, so perhaps it's worth it?

TBH, I suspect that a lot of these allocations can also be turned into constant allocations of reasonable bounds. For instance, I doubt we'll really need to be able to support more than maybe 8 threads for the renderer... probably even less. Such a limit could even be turned into a build-time constant if needed, if we'd want even more flexibility.

Anyway, I thought I'd post them, perhaps someone else has some thoughts here?

Note: these aren't properly tested yet, so please don't merge yet.

yupferris commented 4 years ago

32 bytes isn't very much, so it doesn't seem like a great idea to me on the surface admittedly.

However, one thing that this may help with is removing dependencies. I recall having to shim these at one point, and I remember that coming up again when we added support for more recent VS versions. I don't remember if that was an issue and how we got around that if so?

kusma commented 4 years ago

32 bytes isn't very much, so it doesn't seem like a great idea to me on the surface admittedly.

However, one thing that this may help with is removing dependencies. I recall having to shim these at one point, and I remember that coming up again when we added support for more recent VS versions. I don't remember if that was an issue and how we got around that if so?

Yeah, I'm far from convinced about this PR as well.

The work that sparked this was to get things building on newer VS versions, but we've crossed that bridge already. So these patches aren't directly useful anymore as far as I'm aware.

kusma commented 3 years ago

I think I'll just close this now. Things are fine as-is, I think.