Re: [PATCH v5 2/6] perf record: Get the first sample time and last sample time

From: Jin, Yao
Date: Mon Oct 23 2017 - 22:03:26 EST




On 10/23/2017 11:04 PM, Jiri Olsa wrote:
On Sat, Oct 21, 2017 at 07:27:50AM +0800, Jin Yao wrote:
In perf record, it's walked on all samples yet. So it's very easy to get
the first/last samples and save the time to perf file header via the
function write_sample_time().

In later, perf report/script will fetch the time from perf file header.

Change log:
-----------
v5: There is an issue that the sample walking can only work when
'--buildid-all' is not enabled. So we need to let the walking
be able to work even if '--buildid-all' is enabled and let the
processing skips the dso hit marking for this case.

At first, I want to provide a new option "--record-time-boundaries".
While after consideration, I think a new option is not very
necessary.

v3: Remove the definitions of first_sample_time and last_sample_time
from struct record and directly save them in perf_evlist.

Signed-off-by: Jin Yao <yao.jin@xxxxxxxxxxxxxxx>
---
tools/perf/builtin-record.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index a6cbf16..bd711e8 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -391,8 +391,19 @@ static int process_sample_event(struct perf_tool *tool,
{
struct record *rec = container_of(tool, struct record, tool);
- rec->samples++;
+ if (rec->evlist->first_sample_time == 0)
+ rec->evlist->first_sample_time = sample->time;
+
+ rec->evlist->last_sample_time = sample->time;
+
+ /*
+ * If --buildid-all is given, it marks all DSO regardless of hits,
+ * so no need to process this sample.
+ */
+ if (rec->buildid_all)
+ return 0;
+ rec->samples++;
return build_id__mark_dso_hit(tool, event, sample, evsel, machine);
}
@@ -415,13 +426,6 @@ static int process_buildids(struct record *rec)
*/
symbol_conf.ignore_vmlinux_buildid = true;
- /*
- * If --buildid-all is given, it marks all DSO regardless of hits,
- * so no need to process samples.
- */
- if (rec->buildid_all)
- rec->tool.sample = NULL;

hum, could you still unset the sample if there's no time given?
and keep the speed in this case..

jirka


Hi Jiri,

I check this question again. The '--time' option is for perf report but not for perf record.

For perf record, we have to always walk on all samples to get the time of first sample and the time of last sample whatever buildid_all is enabled or not enabled. So 'rec->tool.sample = NULL' is removed.

Sorry, the previous mail was replied at midnight, I was drowsy. :(

If my answer is correct, I will not send v6. If my understanding is still not correct, please let me know.

Thanks
Jin Yao