From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756032AbdKCQ3q (ORCPT ); Fri, 3 Nov 2017 12:29:46 -0400 Received: from mail.kernel.org ([198.145.29.99]:54532 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752309AbdKCQ3p (ORCPT ); Fri, 3 Nov 2017 12:29:45 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C3D94218B5 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=acme@kernel.org Date: Fri, 3 Nov 2017 13:29:42 -0300 From: Arnaldo Carvalho de Melo To: Jiri Olsa Cc: "Jin, Yao" , jolsa@kernel.org, peterz@infradead.org, mingo@redhat.com, alexander.shishkin@linux.intel.com, Linux-kernel@vger.kernel.org, ak@linux.intel.com, kan.liang@intel.com, yao.jin@intel.com Subject: Re: [PATCH v5 2/6] perf record: Get the first sample time and last sample time Message-ID: <20171103162942.GF3531@kernel.org> References: <1508542074-29483-1-git-send-email-yao.jin@linux.intel.com> <1508542074-29483-3-git-send-email-yao.jin@linux.intel.com> <20171023150436.GA10746@krava> <818cd5bf-fd40-444e-4f4d-c9e25cf90b51@linux.intel.com> <20171024071659.GB27972@krava> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171024071659.GB27972@krava> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.9.1 (2017-09-22) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Tue, Oct 24, 2017 at 09:16:59AM +0200, Jiri Olsa escreveu: > On Tue, Oct 24, 2017 at 10:03:05AM +0800, Jin, Yao wrote: > > SNIP > > > > 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. > > right, I did not realize we store this unconditionaly.. then yes, it's ok And should we store this unconditionally? What this patch is doing is making 'perf record' unconditionally slower so that the generated perf.data file becomes useful for some usecases, but not for all, only people interested in using 'perf report/script --time' will benefit, right? I thought that we could get this sorted out in a different fashion, i.e. getting the first timestamp is easy, even if we don't process build-ids, right? To get the last one we could ask the kernel to insert an extra dummy sample at the end, one that we know the size and thus can to to the end of the file, rewind that size, get the event and parse the sample, agreed? So I suggest that first make this conditional, i.e. 'perf record --timestamps' will enable the logic you implemented, and as a followup, if you agree, add the dummy, known size event at the end, and then even when build-ids are not processed, the cost for getting the timestamps will be next to zero. - Arnaldo - Arnaldo > I think I've already acked this, anyway for the patchset: > > Acked-by: Jiri Olsa > > thanks, > jirka