Set _StaticBufferBuilder capacity via constructor when merging buffers to optimize memory allocations. #166
Conversation
MeltyPlayer
changed the title
MeltyPlayer
changed the title
MeltyPlayer
changed the title
MeltyPlayer
changed the title
|
Hm.... you're adding multiple features in a single PR, I think it would have been better to propose them in separated PRs. Some of the optimizations seems good, but I don't like too much tagging the main projects as x86/x64 because it would prevent running the libraries on ARM cpus. If the purpose is testing, the main library projects shoud not be affected in any way. |
|
Sounds good, I'll go ahead and remove the x86/x64 changes and split out the other changes. |
|
Just keep in mind the less files you change, the easier for me to review, and more chances to accept merging the PR |
…that it's more robust. Revert "Set up platforms for x86 and x64." This reverts commit 2ab2abd. Set up platforms for x86 and x64.
…irectly to the output FileStream rather than first to a MemoryStream and then copying. Optimized some redundant memory allocations in the WavefrontWriter class to avoid OutOfMemory exceptions in 32-bit mode, which fixes some of the existing tests. Made some more memory optimizations in the WavefrontWriter to limit how many objects are created and stored in memory at once.
…issues, and added a test covering this.
…thod so that it's more robust." This reverts commit ed4450a.
… write directly to the output FileStream rather than first to a MemoryStream and then copying." This reverts commit 80d5c3a.
MeltyPlayer
changed the title
|
Done--I've changed this to just make the buffer change and add the test, and pulled out the WavefrontWriter changes into a separate PR. |
|
It almost looks fine. I feel like the memory calculator is not super precise, and I would increase the allocated memory a bit over what's needed, maybe by 1% with something like this: reservedMemory = reservedMemory + (reservedMemory / 100); // increase by 1% The other thing I would request is to move the unit tests from Toolkit to ThirdParty tests. There's already an entry for you. Third party tests help me remember that some features are actually in use by other developers. |
Pulled in new changes from original repo.
… removed from the PR changes.
…y not be marked as a change anymore.
|
Sounds good, I've added the reserved memory change and moved the test. |
I've run into some issues in the past when exporting large GLB/GLTF models in x86 mode, since 32-bit applications only support a max of 2GB of memory. In this PR, I've made some minor optimizations to memory allocations in the GLB/GLTF/OBJ export logic that make it slightly more robust.
Changes:
_StaticBufferBuilderinMergeBuffers()with the total capacity of all of the individual buffers. When adding to a commonListbeyond its capacity, it will double its capacity each time, potentially beyond what is actually needed. This change ensures that theListonly uses the exact capacity it needs, making it slightly more robust to out of memory errors.