Skip to content

adjust for SVN r286524 and SVN r287369 #5890

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

adjust for SVN r286524 and SVN r287369 #5890

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion include/swift/Basic/Timer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ namespace swift {
public:
explicit SharedTimer(StringRef name) {
if (CompilationTimersEnabled == State::Enabled)
Timer.emplace(name, StringRef("Swift compilation"));
Timer.emplace(name, StringRef("Swift compilation"), StringRef("swift"),
Copy link

Choose a reason for hiding this comment

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

I would rather use Timer.emplace(name, name, StringRef("swift"), ...) otherwise all timers will end up with the same description (the human readable output only shows the description and not the name currently).

Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, why the extra wrapping in StringRef(…)?

Copy link

Choose a reason for hiding this comment

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

For all I know the extra StringRef is completely unnecessary here and was just maintained because it was already there before...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I left them there for consistency. The default constructor should work. Id rather get this merged and then follow up with a cleanup patch to remove the explicit construction.

StringRef("swift related timers"));
else
CompilationTimersEnabled = State::Skipped;
}
Expand Down
4 changes: 2 additions & 2 deletions lib/Driver/Compilation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ int Compilation::performJobsImpl() {
}

int Result = EXIT_SUCCESS;
llvm::TimerGroup DriverTimerGroup("Driver Time Compilation");
llvm::TimerGroup DriverTimerGroup("driver", "Driver Compilation Time");
llvm::SmallDenseMap<const Job *, std::unique_ptr<llvm::Timer>, 16>
DriverTimers;

Expand Down Expand Up @@ -465,7 +465,7 @@ int Compilation::performJobsImpl() {
DriverTimers.insert({
BeganCmd,
std::unique_ptr<llvm::Timer>(
new llvm::Timer(OS.str(), DriverTimerGroup))
new llvm::Timer("task", OS.str(), DriverTimerGroup))
Copy link

Choose a reason for hiding this comment

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

I don't know what this code is about, but having a fixed name but a variable description seems odd.

});
DriverTimers[BeganCmd]->startTimer();
}
Expand Down
2 changes: 1 addition & 1 deletion test/Driver/driver-time-compilation.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// RUN: %target-build-swift -parse -driver-time-compilation %s 2>&1 | %FileCheck %s
// RUN: %target-build-swift -parse -driver-time-compilation %s %S/../Inputs/empty.swift 2>&1 | %FileCheck -check-prefix CHECK-MULTIPLE %s

// CHECK: Driver Time Compilation
// CHECK: Driver Compilation Time
// CHECK: Total Execution Time: {{[0-9]+}}.{{[0-9]+}} seconds ({{[0-9]+}}.{{[0-9]+}} wall clock)
// CHECK: ---Wall Time---
// CHECK: --- Name ---
Expand Down