Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize ToFlatOpcString to reduce memory usage and improve performan… #1863

Merged

Conversation

QuocDatHoang
Copy link
Contributor

Optimized the ToFlatOpcString method to address high memory usage and improve performance.

The previous implementation relied on LINQ, which created a significant amount of intermediate collections and dictionaries, leading to inefficiency, especially when working with Word documents containing images.

This update removes unnecessary LINQ operations and reduces memory overhead, replace manual chunking logic with Convert.ToBase64String using Base64FormattingOptions.InsertLineBreaks.

This approach eliminates the need for LINQ-based chunking and aggregation, resulting in cleaner, more efficient code while adhering to the MIME specification for line breaks in Base64 encoding.

A sample benchmark output when working with a 822KB-word document.

image

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…ce by removing excessive LINQ operations and intermediate collections.

Simplify Base64 string chunking using Convert.ToBase64String with InsertLineBreaks, eliminating manual chunking and aligning with MIME specifications.
@QuocDatHoang
Copy link
Contributor Author

@dotnet-policy-service agree

Copy link
Member

@twsouthwick twsouthwick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! I had wanted to optimize this area figuring it was not performant, but didn't know about that enum option. Thanks!

Copy link

github-actions bot commented Feb 4, 2025

Test Results

    70 files  ±0      70 suites  ±0   1h 6m 2s ⏱️ - 3m 55s
 2 041 tests ±0   2 038 ✅ ±0   3 💤 ±0  0 ❌ ±0 
32 418 runs  ±0  32 382 ✅ ±0  36 💤 ±0  0 ❌ ±0 

Results for commit 570aae7. ± Comparison against base commit ed521b6.

@mikeebowen mikeebowen merged commit dc1318f into dotnet:main Feb 4, 2025
22 checks passed
@QuocDatHoang
Copy link
Contributor Author

Thanks @twsouthwick and @mikeebowen!

@QuocDatHoang QuocDatHoang deleted the flatopc-extensions/fix-high-memory-usage branch February 6, 2025 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants