Skip to content

[scudo] Move ALWAYS_INLINE up in tsd_shared's ScopedTSD ctor #81982

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
merged 2 commits into from
Feb 16, 2024

Conversation

fabio-d
Copy link
Contributor

@fabio-d fabio-d commented Feb 16, 2024

Fix for performance regression introduced by #80061 that slowed
down Fuchsia's MallocFree microbenchmark by 3.5 - 8%

To fix a performance regression introduced by llvm#80061
@llvmbot
Copy link
Member

llvmbot commented Feb 16, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Fabio D'Urso (fabio-d)

Changes

This PR should fix the performance regression that was introduced by #80061


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

1 Files Affected:

  • (modified) compiler-rt/lib/scudo/standalone/tsd_shared.h (+2-2)
diff --git a/compiler-rt/lib/scudo/standalone/tsd_shared.h b/compiler-rt/lib/scudo/standalone/tsd_shared.h
index 0ac2be3d4c294d..db5f6ec2094538 100644
--- a/compiler-rt/lib/scudo/standalone/tsd_shared.h
+++ b/compiler-rt/lib/scudo/standalone/tsd_shared.h
@@ -29,7 +29,7 @@ struct TSDRegistrySharedT {
   using ThisT = TSDRegistrySharedT<Allocator, TSDsArraySize, DefaultTSDCount>;
 
   struct ScopedTSD {
-    ScopedTSD(ThisT &TSDRegistry) {
+    ALWAYS_INLINE ScopedTSD(ThisT &TSDRegistry) {
       CurrentTSD = TSDRegistry.getTSDAndLock();
       DCHECK_NE(CurrentTSD, nullptr);
     }
@@ -133,7 +133,7 @@ struct TSDRegistrySharedT {
   }
 
 private:
-  ALWAYS_INLINE TSD<Allocator> *getTSDAndLock() NO_THREAD_SAFETY_ANALYSIS {
+  TSD<Allocator> *getTSDAndLock() NO_THREAD_SAFETY_ANALYSIS {
     TSD<Allocator> *TSD = getCurrentTSD();
     DCHECK(TSD);
     // Try to lock the currently associated context.

@ChiaHungDuan
Copy link
Contributor

Can you add more details about the performance regression in the description? Besides, please do the related change to the tsd_exclusive as well. Thanks

@ChiaHungDuan
Copy link
Contributor

It's better to mention that's the micro benchmarks on Fuchsia so that we will know more context when look back.

BTW, let's do the change for tsd_exclusive as well

@ChiaHungDuan
Copy link
Contributor

It's better to mention that's the micro benchmarks on Fuchsia so that we will know more context when look back.

BTW, let's do the change for tsd_exclusive as well

Sorry my bad. You have updated the tsd_exclusive.

@fabio-d fabio-d merged commit cc67386 into llvm:main Feb 16, 2024
@fabio-d fabio-d deleted the scoped-tsd-fix-performance-regression branch February 16, 2024 18:48
@fabio-d
Copy link
Contributor Author

fabio-d commented Feb 16, 2024

(for future reference, the issue was https://g-issues.fuchsia.dev/issues/323911094)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants