search

bombomby / optick

C++ Profiler For Games
https://optick.dev
MIT License
2.95k stars 296 forks source link

Fix file corruption when using storage upload #160

Closed growlitheharpo closed 2 years ago

growlitheharpo commented 2 years ago

When using the "Report a bug" feature targeting a network storage, we were experiencing file corruption issues when loading back the saved capture:

image

image

image

When debugging the issue, I noticed that the files saved out manually using the Save button vs the file from the Report Bug button were different sizes:

image

I eventually found that this was caused by not calling Dispose on the GZipStream created by Capture.Create. Reading the documentation on GZipStream, it's not totally clear why this becomes necessary, but the issue is a solid repro without the Dispose and I have not been able to repro with it.

image

~Before finding this issue, I also found that the Save button leads to TimeLine.Save/TimeLine.ForEachResponse which appears as if it could save out more data that may be missed in TaskTrackerViewModel.SetGroup. My fix is to introduce a new interface, ISavable. The FrameGroup now has a reference back to the TimeLine as an ISavable and this is used to save the capture if available. I didn't confirm this was a "real" issue, so I can drop this part of the change if desired. If nothing else, it ensures that the two methods of saving follow a more similar code path.~ Dropped with b6ae6f1

bombomby commented 2 years ago

Thanks for the detailed analysis of the problem! I don't think that ISavable part is necessary, but I'd be happy to merge the fix with the Dispose call if you could separate this change out.

growlitheharpo commented 2 years ago

Thanks for the detailed analysis of the problem! I don't think that ISavable part is necessary, but I'd be happy to merge the fix with the Dispose call if you could separate this change out.

Done in the latest version 🙂