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

Expose TimeTraceProfiler for Async Events #83778

Merged
merged 1 commit into from
Mar 19, 2024

Conversation

atetubou
Copy link
Contributor

@atetubou atetubou commented Mar 4, 2024

To avoid issue like #56554 and #83236 due to no guarantees for nested relationships between file level span and syntax tree level span, I'd like to have a timeTraceAsyncProfilerBegin specific to trace handling only Source spans around

if (IncludeLoc.isValid()) {
if (llvm::timeTraceProfilerEnabled()) {
OptionalFileEntryRef FE = SM.getFileEntryRefForID(SM.getFileID(Loc));
llvm::timeTraceProfilerBegin("Source", FE ? FE->getName()
: StringRef("<unknown>"));
}
IncludeStack.push_back(IncludeLoc);
S->DiagnoseNonDefaultPragmaAlignPack(
Sema::PragmaAlignPackDiagnoseKind::NonDefaultStateAtInclude,
IncludeLoc);
}
break;
}
case ExitFile:
if (!IncludeStack.empty()) {
if (llvm::timeTraceProfilerEnabled())
llvm::timeTraceProfilerEnd();
.

This is a preparation PR to do that in following PR.

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 4, 2024

@llvm/pr-subscribers-llvm-support

Author: Takuto Ikuta (atetubou)

Changes

To avoid issue like #56554 and #83236 due to no guarantees for nested relationships between file level span and syntax tree level span, I'd like to have a TimeTraceProfiler specific to trace handling only Source spans around

if (IncludeLoc.isValid()) {
if (llvm::timeTraceProfilerEnabled()) {
OptionalFileEntryRef FE = SM.getFileEntryRefForID(SM.getFileID(Loc));
llvm::timeTraceProfilerBegin("Source", FE ? FE->getName()
: StringRef("<unknown>"));
}
IncludeStack.push_back(IncludeLoc);
S->DiagnoseNonDefaultPragmaAlignPack(
Sema::PragmaAlignPackDiagnoseKind::NonDefaultStateAtInclude,
IncludeLoc);
}
break;
}
case ExitFile:
if (!IncludeStack.empty()) {
if (llvm::timeTraceProfilerEnabled())
llvm::timeTraceProfilerEnd();
.

This is a preparation patch to do that in following PR.


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

2 Files Affected:

  • (modified) llvm/include/llvm/Support/TimeProfiler.h (+6)
  • (modified) llvm/lib/Support/TimeProfiler.cpp (+21)
diff --git a/llvm/include/llvm/Support/TimeProfiler.h b/llvm/include/llvm/Support/TimeProfiler.h
index 454a65f70231f4..cef4110c10deae 100644
--- a/llvm/include/llvm/Support/TimeProfiler.h
+++ b/llvm/include/llvm/Support/TimeProfiler.h
@@ -86,6 +86,9 @@ class raw_pwrite_stream;
 struct TimeTraceProfiler;
 TimeTraceProfiler *getTimeTraceProfilerInstance();
 
+TimeTraceProfiler *newTimeTraceProfiler(unsigned TimeTraceGranularity = 0,
+                                        StringRef ProcName = "");
+
 /// Initialize the time trace profiler.
 /// This sets up the global \p TimeTraceProfilerInstance
 /// variable to be the profiler instance.
@@ -123,9 +126,12 @@ Error timeTraceProfilerWrite(StringRef PreferredFileName,
 void timeTraceProfilerBegin(StringRef Name, StringRef Detail);
 void timeTraceProfilerBegin(StringRef Name,
                             llvm::function_ref<std::string()> Detail);
+void timeTraceProfilerBegin(TimeTraceProfiler *Profiler, StringRef Name,
+                            StringRef Detail);
 
 /// Manually end the last time section.
 void timeTraceProfilerEnd();
+void timeTraceProfilerEnd(TimeTraceProfiler *Profiler);
 
 /// The TimeTraceScope is a helper class to call the begin and end functions
 /// of the time trace profiler.  When the object is constructed, it begins
diff --git a/llvm/lib/Support/TimeProfiler.cpp b/llvm/lib/Support/TimeProfiler.cpp
index 4d625b3eb5b170..9cd5ea6d81758f 100644
--- a/llvm/lib/Support/TimeProfiler.cpp
+++ b/llvm/lib/Support/TimeProfiler.cpp
@@ -293,6 +293,16 @@ void llvm::timeTraceProfilerInitialize(unsigned TimeTraceGranularity,
       TimeTraceGranularity, llvm::sys::path::filename(ProcName));
 }
 
+TimeTraceProfiler *llvm::newTimeTraceProfiler(unsigned TimeTraceGranularity,
+                                              StringRef ProcName) {
+  TimeTraceProfiler *Profiler =
+      new TimeTraceProfiler(TimeTraceGranularity, ProcName);
+  auto &Instances = getTimeTraceProfilerInstances();
+  std::lock_guard<std::mutex> Lock(Instances.Lock);
+  Instances.List.push_back(Profiler);
+  return Profiler;
+}
+
 // Removes all TimeTraceProfilerInstances.
 // Called from main thread.
 void llvm::timeTraceProfilerCleanup() {
@@ -353,7 +363,18 @@ void llvm::timeTraceProfilerBegin(StringRef Name,
     TimeTraceProfilerInstance->begin(std::string(Name), Detail);
 }
 
+void llvm::timeTraceProfilerBegin(TimeTraceProfiler *Profiler, StringRef Name,
+                                  StringRef Detail) {
+  if (Profiler != nullptr)
+    Profiler->begin(std::string(Name), [&]() { return std::string(Detail); });
+}
+
 void llvm::timeTraceProfilerEnd() {
   if (TimeTraceProfilerInstance != nullptr)
     TimeTraceProfilerInstance->end();
 }
+
+void llvm::timeTraceProfilerEnd(TimeTraceProfiler *Profiler) {
+  if (Profiler != nullptr)
+    Profiler->end();
+}

@atetubou atetubou marked this pull request as draft March 4, 2024 08:06
@atetubou atetubou changed the title Expose TimeTraceProfiler for multiple traces Expose TimeTraceProfiler for overlapping spans Mar 4, 2024
@atetubou atetubou marked this pull request as ready for review March 4, 2024 10:17
@atetubou atetubou marked this pull request as draft March 5, 2024 06:59
@atetubou atetubou changed the title Expose TimeTraceProfiler for overlapping spans Expose TimeTraceProfiler for Async Events Mar 6, 2024
@atetubou atetubou marked this pull request as ready for review March 6, 2024 05:53
@atetubou
Copy link
Contributor Author

@ZequanWu could you take a look this?

llvm/lib/Support/TimeProfiler.cpp Outdated Show resolved Hide resolved
90ebde0
Expose TimeTraceProfiler for Async Events

8bf598f71d37f5404671dbb1840c7bed0ed87b42
assert
@atetubou
Copy link
Contributor Author

@MaskRay thank you for taking a look. If this looks good to you, could you merge this PR?
I'll rebase #83961 when this is merged.

@MaskRay MaskRay merged commit e61922d into llvm:main Mar 19, 2024
4 checks passed
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
To avoid issue like llvm#56554
and llvm#83236 due to no
guarantees for nested relationships between file level span and syntax
tree level span, I'd like to have a `timeTraceAsyncProfilerBegin`
specific to trace handling only `Source` spans around

https://github.com/llvm/llvm-project/blob/8715f256911786520bb727ce067098d7082ac45c/clang/lib/Sema/Sema.cpp#L153-L170.

This is a preparation PR to do that in [following
PR](llvm#83961).
MaskRay pushed a commit that referenced this pull request Mar 26, 2024
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.

None yet

3 participants