search

splintermind / Dwarf-Therapist

Maintained branch of the original Dwarf Therapist for Dwarf Fortress.
Other
405 stars 71 forks source link

Make compatible with MSVC #320

Open TV4Fun opened 7 years ago

TV4Fun commented 7 years ago

This fixes compilation with MSVC under windows by removing the VLA from DFInstanceWindows and instead using a unique_ptr to an array on the heap.

Hello71 commented 7 years ago

as I said in the other issue, this is bad because you are doing unnecessary heap allocations in a hot path. apparently that code has always checked len > 1024, so just make the buffer 1024 bytes.

TV4Fun commented 7 years ago

@Hello71, this seems like premature optimization to me. How often is read_string used that an extra heap allocation would actually make that big of a difference? In any case, here is a compromise. We use VLAs when building under GCC, otherwise we use a fixed length of 1024 bytes.

lethosor commented 7 years ago

I would expect it to mainly be used when reading data from DF, maybe 10-100 times per dwarf, so the time it takes to do ~10k allocations probably isn't significant compared to the time it takes to connect to DF and read from it (i.e. I'd expect read_raw to be more of a bottleneck). On the other hand, I could be wrong about how often it's used, and you already have a pretty reasonable maximum length.

Hello71 commented 7 years ago

I mean just use a 1024-byte buffer because of the length check right before.

lethosor commented 7 years ago

There's also a len < 65536 check after that one, apparently - maybe the second should be removed/changed?

Hello71 commented 7 years ago

they've been there for over six years, so...

TV4Fun commented 7 years ago

All of the asserts there are redundant with checks just before them, so I removed them.