This is an archive of the discontinued LLVM Phabricator instance.

[Profile] document %h and %m
ClosedPublic

Authored by davidxl on Jul 20 2016, 2:10 PM.

Details

Summary

Added documentation for %h and %m specifiers. %<n>m specifier which specifies the number of copies is not documented yet (treated as internal for now)

Diff Detail

Repository
rL LLVM

Event Timeline

davidxl updated this revision to Diff 64758.Jul 20 2016, 2:10 PM
davidxl retitled this revision from to [Profile] document %h and %m.
davidxl updated this object.
davidxl added reviewers: vsk, silvas.
davidxl added a subscriber: cfe-commits.
davidxl updated this revision to Diff 64759.Jul 20 2016, 2:12 PM

Fixed a typo

silvas edited edge metadata.Jul 20 2016, 3:37 PM

LGTM with some small wording nits.

We may want to extend this to mention number modifier to %m (e.g. %4m). Perhaps it is better to leave that to more advanced documentation -- your experiments showed that even just 1 merge pool is quite scalable IIRC.

docs/UsersManual.rst
1500 ↗(On Diff #64759)

I would suggest saying Additionally, multiple... instead of just Multiple to highlight that this is an additional behavior that is caused by using %m.

1501 ↗(On Diff #64759)

You probably want to say "raw profiles" instead of just "profile" to clarify that this is a different sense of "merge" than llvm-profdata merge ... (or otherwise clarify).

vsk edited edge metadata.Jul 20 2016, 4:09 PM

LGTM with some small wording nits.

We may want to extend this to mention number modifier to %m (e.g. %4m). Perhaps it is better to leave that to more advanced documentation -- your experiments showed that even just 1 merge pool is quite scalable IIRC.

Actually, I think this is the right place to document the numeric modifier to %m. LGTM with Sean's nits.

davidxl added inline comments.Jul 20 2016, 4:18 PM
docs/UsersManual.rst
1500 ↗(On Diff #64759)

Done.

1501 ↗(On Diff #64759)

Done. I also added another sentence clarifying it.

davidxl updated this revision to Diff 64793.Jul 20 2016, 4:20 PM
davidxl edited edge metadata.

Addressed review comments.

I still think %4m etc is an advanced feature that needs more explanation. We can delay that to a later time.

silvas accepted this revision.Jul 20 2016, 4:32 PM
silvas edited edge metadata.

LGTM (also, another small suggestion).

docs/UsersManual.rst
1502 ↗(On Diff #64793)

"running on the same or different hosts" should probably just be "that share a file system" to clarify that we depend on the filesystem guarantees and aren't doing anything else special.

This revision is now accepted and ready to land.Jul 20 2016, 4:32 PM
This revision was automatically updated to reflect the committed changes.