search

Cheesehyvel / magesim-tbc2

MageSim TBC in C++
MIT License
9 stars 18 forks source link

Use of Dynamic Memory #35

Closed smalls12 closed 2 years ago

smalls12 commented 2 years ago

Hey,

Just stumbled onto your tool from watching a TwitchTV streamer using it.

I really like the use of C/C++ with Emscriptem.

I was just poking around the code and just thought I would pose some questions to you.

Why are you using shared pointers so liberally here when passing them down into other objects?

Seems like something more suitable would be to create on the stack and pass as reference.

https://github.com/Cheesehyvel/magesim-tbc2/blob/15b9e080284dea8516520f6fcba665f34ea89dfe/src/main.cpp#L37

Thanks,

Cheesehyvel commented 2 years ago

I'm not a professional C++ coder so I just took the safe route to avoid any memory leaks.

smalls12 commented 2 years ago

Ah well that works. The alternative being if you make everything on the stack ( ie. not pointers ) there are no leaks as well.

Things like this as well https://github.com/Cheesehyvel/magesim-tbc2/blob/335fda69b727d322f2951af423a022ff4853080a/src/state.h#L23 will undoubtedly be massive performance hits to your simulations.

Shared pointers contain alot of thread safety mechanics which you do not need here in a single threaded application. Allocating memory on the stack in this way takes time and when you do it repetitively it would not scale well. Also passing shared pointers by value can be expensive.

This all being said, the simulation seems to run very quickly anyways. I suppose if you were to ramp it up above 30k sims it would scale worse.

Cheesehyvel commented 2 years ago

Will close this for now. Performance isn't much of an issue at this time.