DO NOT MERGE YET: Rdar 30961871 metrics #8178
Conversation
|
This looks awesome to me, I think it has all the things I want! Can we get an update to: |
| static llvm::Optional<ResourceStats> OwnResourceStats(); | ||
|
|
||
| // \brief Get the cumulative resources used by all children. | ||
| static llvm::Optional<ResourceStats> ChildResourceStats(); |
There was a problem hiding this comment.
Nitpick: Method names should have lowerCamelCase.
| TaskQueue::ChildResourceStats() | ||
| { | ||
| return Optional<ResourceStats>(); | ||
| } |
There was a problem hiding this comment.
You forgot to update the Finished callback in the default TaskQueue implementation.
| /// | ||
| /// \returns a TaskFinishedResponse indicating whether or not execution | ||
| /// should proceed | ||
| typedef std::function<TaskFinishedResponse(ProcessId Pid, StringRef ErrorMsg, | ||
| StringRef Output, StringRef Errors, | ||
| void *Context, Optional<int> Signal)> | ||
| void *Context, Optional<int> Signal, | ||
| llvm::Optional<ResourceStats> Resources)> |
There was a problem hiding this comment.
Nitpick: No "llvm::" needed.
| struct ScalarTraits<llvm::sys::TimePoint<>> { | ||
| static void output(const llvm::sys::TimePoint<> &value, llvm::raw_ostream &os) { | ||
| using namespace std::chrono; | ||
| os << duration_cast<microseconds>(value.time_since_epoch()).count(); |
There was a problem hiding this comment.
time_since_epoch isn't necessarily the same on all systems. We should probably pick our own epoch.
| void markTransitive(SmallVectorImpl<const void *> &visited, | ||
| const void *node, MarkTracerImpl *tracer = nullptr); | ||
| const void *node, | ||
| CompilationCounters &counters, |
There was a problem hiding this comment.
Can you put this inside MarkTracer instead? Right now DependencyGraphImpl is still pretty generic and I like that we'd be able to break it out for some other purpose if we had to.
| void | ||
| DependencyGraphImpl::countMarking(CompilationCounters &counters, | ||
| DependencyMaskTy kinds, bool isCascading) { | ||
| if (isCascading) { |
There was a problem hiding this comment.
This data is measuring "dependency edges actually traversed during a rebuild", right? Is that the right thing to be measuring? (As opposed to "total kinds of dependency edges", or "max fan-out from one changed file, regardless of edges taken to get there".)
|
Closing this out in favour of #8477 |
This set of changes augments ParseableOutput with a bit of cheap metrics-reporting machinery,
eventually destined for some form of telemetry Xcode-side (work there is still TBD, so don't merge
yet!)
The metrics come in two forms: a general rusage (user/sys/rss) dictionary reported for each
subprocess the driver runs, and a final set of counters representing jobs run and dependency
types that caused them (along with a cumulative rusage for the whole child process tree).
Feedback welcome. I expect this isn't entirely right but I tried a few other arrangements and this
seems like the least-intrusive way of getting a small set of lightweight cost signals out of compilers
in the field.
rdar://30961871