Skip to content

Conversation

@kazutakahirata
Copy link
Contributor

A lambda is a lot shorter than std::bind here.

A lambda is a lot shorter than std::bind here.
@kazutakahirata kazutakahirata requested a review from snehasish June 19, 2025 18:15
@llvmbot llvmbot added the PGO Profile Guided Optimizations label Jun 19, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 19, 2025

@llvm/pr-subscribers-pgo

Author: Kazu Hirata (kazutakahirata)

Changes

A lambda is a lot shorter than std::bind here.


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

1 Files Affected:

  • (modified) llvm/include/llvm/ProfileData/MemProfReader.h (+1-2)
diff --git a/llvm/include/llvm/ProfileData/MemProfReader.h b/llvm/include/llvm/ProfileData/MemProfReader.h
index 4d41d05b1457c..25578ecd06f12 100644
--- a/llvm/include/llvm/ProfileData/MemProfReader.h
+++ b/llvm/include/llvm/ProfileData/MemProfReader.h
@@ -62,8 +62,7 @@ class MemProfReader {
       return make_error<InstrProfError>(instrprof_error::eof);
 
     if (Callback == nullptr)
-      Callback =
-          std::bind(&MemProfReader::idToFrame, this, std::placeholders::_1);
+      Callback = [&](FrameId Id) { return idToFrame(Id); };
 
     CallStackIdConverter<decltype(MemProfData.CallStacks)> CSIdConv(
         MemProfData.CallStacks, Callback);

Copy link

@snehasish snehasish left a comment

Choose a reason for hiding this comment

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

lgtm

@firewave
Copy link

Using std::mem_fn() would be even shorter.

@kazutakahirata
Copy link
Contributor Author

Using std::mem_fn() would be even shorter.

Thanks for a comment! IIUC, with std::mem_fn, we would do something like:

Callback = [&](FrameId Id) { return std::mem_fn(&MemProfReader::idToFrame)(this, Id); };

At that point, it's longer than:

Callback = [&](FrameId Id) { return idToFrame(Id); };

I don't think we can "package" this into std::mem_fn.

@kazutakahirata kazutakahirata merged commit 03692aa into llvm:main Jun 19, 2025
9 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_20250619_std_bind_memprof branch June 19, 2025 20:18
@firewave
Copy link

I thought it is just

Callback = std::mem_fn(&MemProfReader::idToFrame);

But I did not consider that you have to pass the object you are calling on into the call. Sorry for the noise.

@snehasish
Copy link

But I did not consider that you have to pass the object you are calling on into the call. Sorry for the noise.

TIL about std::mem_fn from your comment, thanks!

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

Labels

PGO Profile Guided Optimizations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants