Profile stats #14703

graydon · 2018-02-18T08:33:38Z

Followup to #14695 with a single additional feature, commit ea2e87f, that adds a couple banks of hierarchical profilers to the -stats-output-dir / -trace-stats-events machinery. The user need only pass -profile-stats-events or -profile-stats-entities (along with -stats-output-dir) to get the full feature, so it's still quite easy to explain, capture, transport, etc.

The additional output is a subdirectory full of plain-text hierarchical profiles in the input format to flamegraph.pl, each of which shows time or event-counts arranged into pseudo-stacks formed by the entry/exit events of the FrontendStatsTracer objects, optionally subdivided by source entity (though that makes the profile quite a bit more fine-grained -- it also removes the mystery of which call to "typecheck-decl" is which!).

Here's a (github-sanitized, script-disabled so only half-useful, but you can still get tooltips) version showing the Sema.NumGenericSignatureBuilders counter broken down by source entity in a single file from the Plank project, as an example:

I'll push a few more tests next week, just wanted to get this posted for any early feedback.

graydon · 2018-02-18T09:41:06Z

@swift-ci please test

swift-ci · 2018-02-18T10:31:47Z

Build failed
Swift Test Linux Platform
Git Sha - 59a83270923a85e4fa36811a85995ff8c59362ee

swift-ci · 2018-02-18T11:04:56Z

Build failed
Swift Test OS X Platform
Git Sha - 59a83270923a85e4fa36811a85995ff8c59362ee

davidungar · 2018-02-18T15:24:56Z

Sounds great! Can’t wait to try it out. - David

…

On Feb 18, 2018, at 2:41 AM, Graydon Hoare ***@***.***> wrote: @swift-ci please test — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

graydon · 2018-02-19T18:31:32Z

@swift-ci please test

swift-ci · 2018-02-19T18:33:13Z

Build failed
Swift Test Linux Platform
Git Sha - 59a83270923a85e4fa36811a85995ff8c59362ee

swift-ci · 2018-02-19T18:33:17Z

Build failed
Swift Test OS X Platform
Git Sha - 59a83270923a85e4fa36811a85995ff8c59362ee

jrose-apple

I don't see anything that looks unreasonable here, although I'm not super happy with all the macros.

jrose-apple · 2018-02-20T18:25:01Z

include/swift/Basic/Statistic.h

  llvm::TimeRecord StartedTime;
  std::unique_ptr<llvm::NamedRegionTimer> Timer;
  SourceManager *SourceMgr;
  clang::SourceManager *ClangSourceMgr;
  std::unique_ptr<AlwaysOnDriverCounters> DriverCounters;
  std::unique_ptr<AlwaysOnFrontendCounters> FrontendCounters;
  std::unique_ptr<AlwaysOnFrontendCounters> LastTracedFrontendCounters;
-  std::vector<FrontendStatsEvent> FrontendStatsEvents;
+  std::unique_ptr<std::vector<FrontendStatsEvent>> FrontendStatsEvents;


Why the extra indirection?

Congruent with the other machinery where we test if it's active based on the presence/absence of the supporting data structure (i.e. nullptr == disabled); this PR changes the test for tracing (vs. profiling which also uses the LastTracedFrontendCounters) from testing LastTracedFrontendCounters to testing FrontendStatsEvents, so the latter moves to a thing-that-can-be-nullptr. I suppose I could also split enabled/disabled signalling into a set of boolean flags if you prefer. That would make it a bit more obvious.

I'm fine with an explicit test rather than relying on an empty state, but is there a reason to use unique_ptr over Optional?

Nope, just muscle memory from the days before a reliable C++ Optional in my codebase.

Optional is good! I'll switch 'em over.

jrose-apple · 2018-02-20T18:26:43Z

lib/Basic/Statistic.cpp

+    SmallString<256> Path(Dirname);
+    llvm::sys::path::append(Path, Filename);
+    std::error_code EC;
+    raw_fd_ostream Stream(Path, EC, fs::F_Append | fs::F_Text); \


Stray \ here.

whoops! fixing

graydon · 2018-02-20T21:06:50Z

@jrose-apple Pushed requested changes (removed a bunch of macros too) and added test.

graydon · 2018-02-20T21:06:56Z

@swift-ci please test

swift-ci · 2018-02-20T21:07:27Z

Build failed
Swift Test OS X Platform
Git Sha - c054edea2b62be6058ec3c15a9ff7cc04e7cc239

swift-ci · 2018-02-20T21:08:36Z

Build failed
Swift Test Linux Platform
Git Sha - c054edea2b62be6058ec3c15a9ff7cc04e7cc239

jrose-apple · 2018-02-20T21:37:14Z

Note that I left other comments on the other PR.

graydon · 2018-02-20T21:38:46Z

Oops, missed them, sorry; fixing now.

graydon · 2018-02-21T07:08:52Z

@jrose-apple pushed update with fixes to both this branch and the prefix (and closing out that PR since you seem roughly ok with this extension).

graydon · 2018-02-21T07:10:29Z

@swift-ci please test

swift-ci · 2018-02-21T07:10:56Z

Build failed
Swift Test OS X Platform
Git Sha - 9d7b887d521b8452175c49162951e99593f7319f

swift-ci · 2018-02-21T07:12:13Z

Build failed
Swift Test Linux Platform
Git Sha - 9d7b887d521b8452175c49162951e99593f7319f

jrose-apple · 2018-02-21T16:46:39Z

include/swift/Basic/Statistic.h

+  llvm::Optional<AlwaysOnDriverCounters> DriverCounters;
+  llvm::Optional<AlwaysOnFrontendCounters> FrontendCounters;
+  llvm::Optional<AlwaysOnFrontendCounters> LastTracedFrontendCounters;
+  llvm::Optional<std::vector<FrontendStatsEvent>> FrontendStatsEvents;


Nitpick: No need for the llvm:: prefix if you import swift/Basic/LLVM.h

I never knew! Nice.

jrose-apple · 2018-02-21T16:49:34Z

lib/Basic/Statistic.cpp

-          NowUS, LiveUS, IsEntry, T.EventName, name,          \
-            delta, total, T.Entity, T.Formatter});            \
-    }                                                         \
+#define FRONTEND_STATISTIC(TY, NAME)                            \


This was actually the macro that scared me most, but I guess it's hard to factor out.

#define FRONTEND_STATISTIC(TY, NAME) \ handleStatistic(#TY "." #NAME, C.NAME, LastTracedFrontendCounters->NAME, EventProfilers->NAME, EntityProfilers->NAME);

…well, maybe?

Mm, fair. Lemme see what I can do.

Yep, this cleaned up nicely. Thanks!

graydon · 2018-02-22T00:23:18Z

@swift-ci please test and merge

graydon · 2018-02-22T01:20:25Z

@swift-ci please test and merge

graydon requested a review from DougGregor February 18, 2018 08:33

graydon force-pushed the profile-stats branch from 7fb7384 to 59a8327 Compare February 18, 2018 09:30

graydon force-pushed the profile-stats branch from 59a8327 to c054ede Compare February 19, 2018 18:30

graydon requested a review from jrose-apple February 20, 2018 18:13

jrose-apple reviewed Feb 20, 2018

View reviewed changes

graydon force-pushed the profile-stats branch from c054ede to 9d7b887 Compare February 20, 2018 21:05

graydon force-pushed the profile-stats branch from 9d7b887 to 35c4942 Compare February 21, 2018 07:07

graydon mentioned this pull request Feb 21, 2018

Stats tracer timer work #14695

Closed

jrose-apple approved these changes Feb 21, 2018

View reviewed changes

graydon added 4 commits February 21, 2018 14:49

[Stats] Improve stats tracing readability a little.

6f46362

[Stats] Simplify FrontendStatsTracer uses and formatter-definitions.

76b82ac

[Stats] Add support for tracing protocol conformances.

5f01c19

[Stats] Add a stats tracer to check-conformance phase.

c968670

graydon added 5 commits February 21, 2018 14:49

[Stats] Expand -trace-stats-events test to cover recent work.

9446d51

[Stats] Add a recursive-timers registry to UnifiedStatsReporter.

deabc3f

[Stats] Replace RecursiveSharedTimer with FrontendStatsTracer.

8192759

[Stats] Replace nonstandard SharedTimers with FrontendStatsTracers.

dd85e94

[Stats] Permit negative counters and deltas.

7fe97c7

graydon force-pushed the profile-stats branch from 35c4942 to 757aba6 Compare February 22, 2018 00:05

graydon added 2 commits February 21, 2018 16:22

[Stats] Add -profile-stats-{events,entities}

628c446

[Stats] Add a test for -profile-stats-{events,entities}

9d34041

graydon force-pushed the profile-stats branch from 757aba6 to 9d34041 Compare February 22, 2018 00:23

swift-ci merged commit 786cfc5 into swiftlang:master Feb 22, 2018

Profile stats #14703

Profile stats #14703

Uh oh!

Conversation

graydon commented Feb 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

graydon commented Feb 18, 2018

Uh oh!

swift-ci commented Feb 18, 2018

Uh oh!

swift-ci commented Feb 18, 2018

Uh oh!

davidungar commented Feb 18, 2018 via email

Uh oh!

graydon commented Feb 19, 2018

Uh oh!

swift-ci commented Feb 19, 2018

Uh oh!

swift-ci commented Feb 19, 2018

Uh oh!

jrose-apple left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

graydon commented Feb 20, 2018

Uh oh!

graydon commented Feb 20, 2018

Uh oh!

swift-ci commented Feb 20, 2018

Uh oh!

swift-ci commented Feb 20, 2018

Uh oh!

jrose-apple commented Feb 20, 2018

Uh oh!

graydon commented Feb 20, 2018

Uh oh!

graydon commented Feb 21, 2018

Uh oh!

graydon commented Feb 21, 2018

Uh oh!

swift-ci commented Feb 21, 2018

Uh oh!

swift-ci commented Feb 21, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

graydon commented Feb 22, 2018

Uh oh!

graydon commented Feb 22, 2018

Uh oh!

Uh oh!

graydon commented Feb 18, 2018 •

edited

Loading