[PATCH] Add documentation for sample profiling support.

Richard Smith richard at metafoo.co.uk
Mon Apr 21 14:41:15 PDT 2014


  Mostly copy-editing, but some questions too.


================
Comment at: docs/UsersManual.rst:1073
@@ +1072,3 @@
+hardware counters, while your application executes. They are typically
+very efficient and do not incur in a large runtime overhead. The
+sample data collected by the profiler can be used during compilation
----------------
Should this be "do not incur a large runtime overhead" ?

================
Comment at: docs/UsersManual.rst:1075
@@ +1074,3 @@
+sample data collected by the profiler can be used during compilation
+to determine what are the most executed areas of the code.
+
----------------
"to determine what the most executed ares of the code are."

================
Comment at: docs/UsersManual.rst:1078
@@ +1077,3 @@
+In particular, sample profilers can provide execution counts for all
+instructions in the code, information on branches taken and function
+invocation. The compiler can use this information in its optimization
----------------
"and" instead of ",".

================
Comment at: docs/UsersManual.rst:1083
@@ +1082,3 @@
+basic blocks. Knowing that a function ``foo`` is called more
+frequently than another ``bar`` helps the inliner.
+
----------------
"another function ``bar``"

================
Comment at: docs/UsersManual.rst:1092
@@ +1091,3 @@
+   usual build flags that you always build your application with. The only
+   requirement is that you add ``-gline-tables-ony`` or ``-g`` to the
+   command line. This is important for the profiler to be able to map
----------------
"``-gline-tables-only``"

================
Comment at: docs/UsersManual.rst:1131-1133
@@ +1130,5 @@
+
+4. Build the code again using the collected profile. This step feeds
+   the profile back to the optimizers. This should result in a binary
+   that executes faster than the original one.
+
----------------
Are you required to use the exact same arguments that you used before, other than the `-fprofile-sample-use=` argument? Either way, we should say so here.

================
Comment at: docs/UsersManual.rst:1165
@@ +1164,3 @@
+at the prologue of the function (second number). This head sample
+count provides an indicator of how frequent is the function invoked.
+
----------------
"how frequently the function is invoked."

================
Comment at: docs/UsersManual.rst:1164
@@ +1163,3 @@
+function (first number), and the total number of samples accumulated
+at the prologue of the function (second number). This head sample
+count provides an indicator of how frequent is the function invoked.
----------------
Should "at the prologue" be "in the prologue"?

================
Comment at: docs/UsersManual.rst:1176-1178
@@ +1175,5 @@
+
+b. [OPTIONAL] Discriminator. This is used if the sampled program
+   was compiled with DWARF discriminator support
+   (http://wiki.dwarfstd.org/index.php?title=Path_Discriminators)
+
----------------
... and what is it, in that case? At a guess: "If present, this is a decimal integer representing the value of the DWARF discriminator register."

================
Comment at: docs/UsersManual.rst:1173-1174
@@ +1172,4 @@
+   always relative to the line where symbol of the function is
+   defined. So, if the function has its header at line 280, the offset
+   13 is at line 293 in the file.
+
----------------
What happens if the line is in a different file (eg through macro expansion or inlining)?

================
Comment at: docs/UsersManual.rst:1180-1181
@@ +1179,4 @@
+
+c. Number of samples. This is the number of samples collected by
+   the profiler at this source location.
+
----------------
Decimal integer? Floating point? Something else? Is scientific notation OK? Is hexadecimal OK?

================
Comment at: docs/UsersManual.rst:1149-1150
@@ +1148,4 @@
+which correspond to each of the functions executed at runtime. Each
+section has the following format (taken from
+https://github.com/google/autofdo/blob/master/profile_writer.h):
+
----------------
Is the whitespace required to be a single space, or can it be any sequence of horizontal whitespace? Is any other whitespace allowed than that listed here?

================
Comment at: docs/UsersManual.rst:1193-1194
@@ +1192,4 @@
+   The above means that at relative line offset 130 there is a call
+   instruction that calls one of ``foo()``, ``bar()`` and ``baz()``.
+   With ``baz()`` being the relatively more frequent call target.
+
----------------
". With" -> ", with"
"frequent call" -> "frequently called"


http://reviews.llvm.org/D3402






More information about the cfe-commits mailing list