Skip to content

[ThinLTO] Use DenseMap for OidGuidMap #107725

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

Merged

Conversation

kazutakahirata
Copy link
Contributor

We use OidGuidMap only to map an old GUID to a new one. We don't use
std::set's strengths like iterators staying valid or the ability to
traverse in a sorted order.

As a data point, during the ThinLTO indexing step of a large
application of ours, we create 440,000 mappings. Our memory profiler
reports reduction of 127MB in the peak memory usage (out of 4.991GB),
which is bigger than expected savings most likely due to some noise.
The savings should be about 8MB at the max load factor of DenseMap.

We use OidGuidMap only to map an old GUID to a new one.  We don't use
std::set's strengths like iterators staying valid or the ability to
traverse in a sorted order.

As a data point, during the ThinLTO indexing step of a large
application of ours, we create 440,000 mappings.  Our memory profiler
reports reduction of 127MB in the peak memory usage (out of 4.991GB),
which is bigger than expected savings most likely due to some noise.
The savings should be about 8MB at the max load factor of DenseMap.
@llvmbot
Copy link
Member

llvmbot commented Sep 7, 2024

@llvm/pr-subscribers-llvm-ir

Author: Kazu Hirata (kazutakahirata)

Changes

We use OidGuidMap only to map an old GUID to a new one. We don't use
std::set's strengths like iterators staying valid or the ability to
traverse in a sorted order.

As a data point, during the ThinLTO indexing step of a large
application of ours, we create 440,000 mappings. Our memory profiler
reports reduction of 127MB in the peak memory usage (out of 4.991GB),
which is bigger than expected savings most likely due to some noise.
The savings should be about 8MB at the max load factor of DenseMap.


Full diff: https://github.com/llvm/llvm-project/pull/107725.diff

1 Files Affected:

  • (modified) llvm/include/llvm/IR/ModuleSummaryIndex.h (+1-1)
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index af3734cc2c54b5..dae208d0b132e4 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1343,7 +1343,7 @@ class ModuleSummaryIndex {
 
   /// Mapping from original ID to GUID. If original ID can map to multiple
   /// GUIDs, it will be mapped to 0.
-  std::map<GlobalValue::GUID, GlobalValue::GUID> OidGuidMap;
+  DenseMap<GlobalValue::GUID, GlobalValue::GUID> OidGuidMap;
 
   /// Indicates that summary-based GlobalValue GC has run, and values with
   /// GVFlags::Live==false are really dead. Otherwise, all values must be

@kazutakahirata kazutakahirata merged commit 482e7dc into llvm:main Sep 8, 2024
10 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_thinlto_DenseMap_OidGuidMap branch September 8, 2024 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants