Optimize DepotManifest Serialization #1337
Conversation
Replaced List<byte[]> with HashSet<byte[]> using a custom ChunkIdComparer for efficient chunk ID uniqueness checks
|
Why does it even need to check for duplicates? |
|
I'm not entirely sure why the original author included the duplicate checks—probably had a good reason. I just worked on speeding up the process they set up. 🤷♂️ |
|
BTW, I'm thinking of making the |
Because Valve's manifest format stores the number of unique chunks, I just followed it. |
I checked the original PR and I don't know why I didn't make Serialize public, to be honest. |
| Serialize( fs ); // Directly pass the FileStream to the Serialize method | ||
| return true; // If serialization completes without throwing an exception, return true | ||
| } | ||
| catch ( Exception ) |
There was a problem hiding this comment.
Are we expecting any exceptions, why the catch?
There was a problem hiding this comment.
@xPaw Thanks for pointing that out! While Serialize itself might be safe, I was thinking about scenarios where the provided filename could cause problems:
- A user accidentally provides a file path with a non-existent directory (leading to a
DirectoryNotFoundException). - The application might not have write permissions to the specified location (resulting in an
UnauthorizedAccessException). - External factors like a full disk could also cause issues during file writing (
IOException).
Do you think it would be helpful to handle these cases explicitly for better fault tolerance?
This PR builds on PR #1113, introducing a
HashSetwithChunkIdComparerto manage unique chunks inDepotManifestserialization, significantly reducing processing time.My comment
Changes:
List<byte[]>withHashSet<byte[]>for unique chunks.ChunkIdComparerfor efficient byte array comparison.Impact:
This optimization streamlines serialization, especially for large manifests, enhancing performance without data integrity loss.