All of lore.kernel.org
 help / color / mirror / Atom feed
From: Adrian Hunter <adrian.hunter@intel.com>
To: Chris Phlipot <cphlipot0@gmail.com>,
	acme@kernel.org, mingo@redhat.com, peterz@infradead.org
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/5] perf script: extend db-export api to include callchains for samples
Date: Fri, 22 Apr 2016 10:56:04 +0300	[thread overview]
Message-ID: <5719D914.2000109@intel.com> (raw)
In-Reply-To: <1461056164-14914-2-git-send-email-cphlipot0@gmail.com>

On 19/04/16 11:56, Chris Phlipot wrote:
> The current implementation of the python database export API only
> includes call path information when using some form of call/return
> tracing, but is unable to do so when sampling.
> 
> The following API extensions allow exporting of data collected by
> perf record when using --call-graph.
> 
> The additions to the python api include the following:
> - add call_path_id to sample_table to allow association of samples
>   with call paths
> 
> - add dso_id to call_path_table to more closely align the data with that
>   of a callchain_node

The call_paths table already has symbol_id which belongs uniquely to a DSO,
so why do we need dso_id as well?

> 
> db-export and trace-script-python were both extended to accomidate the API

accomidate -> accommodate

> changes listed above.
> 
> Thread-stack's functionality was expanded to allow storage and exporting
> of callchains that result from individual samples.
> 
> - Introduced a new static function (thread_stack__process_callchain) to
>   resolve call paths using the existing callchain resolution provided by
>   thread__resolve_callchain
> 
> - The existing call_path tree in call_return_processor is used for storing
>   the data from the resolved callchain.
> 
> - Call_return_processor was also extended to allow the ability to export
>   full call paths in addtion to the existing individual call/return pairs,
>   since call/return pairs are not available when doing sampling.

Why do you need a callback?  Seems like the only thing you need from
thread-stack.c is the call path tree.  You could move that to its own .c/.h
files and then process the call chain in db-export.c

Also a list of changes like the one above heavily implies you are not
obeying the one patch == one change rule.  Please try to make patches that
only do one thing and also run checkpatch.

If you don't mind, I'll let you respond to my comments before I comment on
any other patches.

  reply	other threads:[~2016-04-22  7:59 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-19  8:56 [PATCH 1/5] perf tools: fix incorrect ordering of callchain entries Chris Phlipot
2016-04-19  8:56 ` [PATCH 2/5] perf script: extend db-export api to include callchains for samples Chris Phlipot
2016-04-22  7:56   ` Adrian Hunter [this message]
2016-04-23  4:41     ` Chris Phlipot
2016-04-28  8:36       ` Chris Phlipot
2016-04-19  8:56 ` [PATCH 3/5] perf script: fix postgresql ubuntu install instructions Chris Phlipot
2016-04-19 13:41   ` Arnaldo Carvalho de Melo
2016-04-23 13:03   ` [tip:perf/core] perf script: Fix " tip-bot for Chris Phlipot
2016-04-19  8:56 ` [PATCH 4/5] perf script: add option to control callchain export from python Chris Phlipot
2016-04-19  8:56 ` [PATCH 5/5] perf script: Update export-to-postgresql to support callchains Chris Phlipot
2016-04-22  7:55 ` [PATCH 1/5] perf tools: fix incorrect ordering of callchain entries Adrian Hunter
2016-04-22  7:59   ` Adrian Hunter
2016-04-23  3:35     ` Chris Phlipot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5719D914.2000109@intel.com \
    --to=adrian.hunter@intel.com \
    --cc=acme@kernel.org \
    --cc=cphlipot0@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.