* Re: [PATCH] perf util: Join four conditions in two in perf_evlist__create_maps().
2013-11-27 22:23 [PATCH] perf util: Join four conditions in two in perf_evlist__create_maps() Dongsheng Yang
@ 2013-11-27 9:43 ` Adrian Hunter
2013-11-27 22:52 ` Dongsheng Yang
0 siblings, 1 reply; 5+ messages in thread
From: Adrian Hunter @ 2013-11-27 9:43 UTC (permalink / raw)
To: Dongsheng Yang; +Cc: linux-kernel, dsahern, jolsa, namhyung, acme, mingo
On 28/11/13 00:23, Dongsheng Yang wrote:
> There are four condition branchs in perf_evlist__create_maps, but they
> only do two kinds of thing. 1st and 4th call cpu_map__new(), and the others
> call cpu_map__dummy_new(). This patch joins them in two branchs, decreasing
> the number of branch and removing some duplicated code.
That code has changed again. See:
http://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/log/?h=perf/core
>
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
> tools/perf/util/evlist.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index bbc746a..2258eb4 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -819,11 +819,9 @@ int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
> if (evlist->threads == NULL)
> return -1;
>
> - if (target->force_per_cpu)
> - evlist->cpus = cpu_map__new(target->cpu_list);
> - else if (target__has_task(target))
> - evlist->cpus = cpu_map__dummy_new();
> - else if (!target__has_cpu(target) && !target->uses_mmap)
> + if (!target->force_per_cpu &&
> + (target__has_task(target) ||
> + (!target__has_cpu(target) && !target->uses_mmap)))
> evlist->cpus = cpu_map__dummy_new();
> else
> evlist->cpus = cpu_map__new(target->cpu_list);
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf util: Join four conditions in two in perf_evlist__create_maps().
2013-11-27 22:52 ` Dongsheng Yang
@ 2013-11-27 10:49 ` Adrian Hunter
2013-11-28 0:36 ` Dongsheng Yang
0 siblings, 1 reply; 5+ messages in thread
From: Adrian Hunter @ 2013-11-27 10:49 UTC (permalink / raw)
To: Dongsheng Yang; +Cc: linux-kernel, dsahern, jolsa, namhyung, acme, mingo
On 28/11/13 00:52, Dongsheng Yang wrote:
> On 11/27/2013 04:43 AM, Adrian Hunter wrote:
>> On 28/11/13 00:23, Dongsheng Yang wrote:
>>> There are four condition branchs in perf_evlist__create_maps, but they
>>> only do two kinds of thing. 1st and 4th call cpu_map__new(), and the others
>>> call cpu_map__dummy_new(). This patch joins them in two branchs, decreasing
>>> the number of branch and removing some duplicated code.
>> That code has changed again. See:
>>
>> http://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/log/?h=perf/core
>>
>>
>
> Hi Adrian!
> Thanx for your reminding. Yes, a more checking is added into the 1st
> branch.
> And I think we still can join them in two conditions.
>
> Do you think this idea is ok to you? If so, I will update my patch base on
> the latest code.
Personally I like code that is easy to read, and I don't think
combining the conditions does that. But you could set a bool
and use that to decide what to call.
>
> Yang
>>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>>> ---
>>> tools/perf/util/evlist.c | 8 +++-----
>>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>>> index bbc746a..2258eb4 100644
>>> --- a/tools/perf/util/evlist.c
>>> +++ b/tools/perf/util/evlist.c
>>> @@ -819,11 +819,9 @@ int perf_evlist__create_maps(struct perf_evlist
>>> *evlist, struct target *target)
>>> if (evlist->threads == NULL)
>>> return -1;
>>> - if (target->force_per_cpu)
>>> - evlist->cpus = cpu_map__new(target->cpu_list);
>>> - else if (target__has_task(target))
>>> - evlist->cpus = cpu_map__dummy_new();
>>> - else if (!target__has_cpu(target) && !target->uses_mmap)
>>> + if (!target->force_per_cpu &&
>>> + (target__has_task(target) ||
>>> + (!target__has_cpu(target) && !target->uses_mmap)))
>>> evlist->cpus = cpu_map__dummy_new();
>>> else
>>> evlist->cpus = cpu_map__new(target->cpu_list);
>>>
>>
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] perf util: Join four conditions in two in perf_evlist__create_maps().
@ 2013-11-27 22:23 Dongsheng Yang
2013-11-27 9:43 ` Adrian Hunter
0 siblings, 1 reply; 5+ messages in thread
From: Dongsheng Yang @ 2013-11-27 22:23 UTC (permalink / raw)
To: linux-kernel
Cc: dsahern, adrian.hunter, jolsa, namhyung, acme, mingo,
Dongsheng Yang
There are four condition branchs in perf_evlist__create_maps, but they
only do two kinds of thing. 1st and 4th call cpu_map__new(), and the others
call cpu_map__dummy_new(). This patch joins them in two branchs, decreasing
the number of branch and removing some duplicated code.
Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
tools/perf/util/evlist.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index bbc746a..2258eb4 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -819,11 +819,9 @@ int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
if (evlist->threads == NULL)
return -1;
- if (target->force_per_cpu)
- evlist->cpus = cpu_map__new(target->cpu_list);
- else if (target__has_task(target))
- evlist->cpus = cpu_map__dummy_new();
- else if (!target__has_cpu(target) && !target->uses_mmap)
+ if (!target->force_per_cpu &&
+ (target__has_task(target) ||
+ (!target__has_cpu(target) && !target->uses_mmap)))
evlist->cpus = cpu_map__dummy_new();
else
evlist->cpus = cpu_map__new(target->cpu_list);
--
1.8.2.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] perf util: Join four conditions in two in perf_evlist__create_maps().
2013-11-27 9:43 ` Adrian Hunter
@ 2013-11-27 22:52 ` Dongsheng Yang
2013-11-27 10:49 ` Adrian Hunter
0 siblings, 1 reply; 5+ messages in thread
From: Dongsheng Yang @ 2013-11-27 22:52 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-kernel, dsahern, jolsa, namhyung, acme, mingo
On 11/27/2013 04:43 AM, Adrian Hunter wrote:
> On 28/11/13 00:23, Dongsheng Yang wrote:
>> There are four condition branchs in perf_evlist__create_maps, but they
>> only do two kinds of thing. 1st and 4th call cpu_map__new(), and the others
>> call cpu_map__dummy_new(). This patch joins them in two branchs, decreasing
>> the number of branch and removing some duplicated code.
> That code has changed again. See:
>
> http://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/log/?h=perf/core
>
Hi Adrian!
Thanx for your reminding. Yes, a more checking is added into the
1st branch.
And I think we still can join them in two conditions.
Do you think this idea is ok to you? If so, I will update my patch base
on the latest code.
Yang
>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>> ---
>> tools/perf/util/evlist.c | 8 +++-----
>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>> index bbc746a..2258eb4 100644
>> --- a/tools/perf/util/evlist.c
>> +++ b/tools/perf/util/evlist.c
>> @@ -819,11 +819,9 @@ int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
>> if (evlist->threads == NULL)
>> return -1;
>>
>> - if (target->force_per_cpu)
>> - evlist->cpus = cpu_map__new(target->cpu_list);
>> - else if (target__has_task(target))
>> - evlist->cpus = cpu_map__dummy_new();
>> - else if (!target__has_cpu(target) && !target->uses_mmap)
>> + if (!target->force_per_cpu &&
>> + (target__has_task(target) ||
>> + (!target__has_cpu(target) && !target->uses_mmap)))
>> evlist->cpus = cpu_map__dummy_new();
>> else
>> evlist->cpus = cpu_map__new(target->cpu_list);
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf util: Join four conditions in two in perf_evlist__create_maps().
2013-11-27 10:49 ` Adrian Hunter
@ 2013-11-28 0:36 ` Dongsheng Yang
0 siblings, 0 replies; 5+ messages in thread
From: Dongsheng Yang @ 2013-11-28 0:36 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-kernel, dsahern, jolsa, namhyung, acme, mingo
On 11/27/2013 05:49 AM, Adrian Hunter wrote:
> On 28/11/13 00:52, Dongsheng Yang wrote:
>> On 11/27/2013 04:43 AM, Adrian Hunter wrote:
>>> On 28/11/13 00:23, Dongsheng Yang wrote:
>>>> There are four condition branchs in perf_evlist__create_maps, but they
>>>> only do two kinds of thing. 1st and 4th call cpu_map__new(), and the others
>>>> call cpu_map__dummy_new(). This patch joins them in two branchs, decreasing
>>>> the number of branch and removing some duplicated code.
>>> That code has changed again. See:
>>>
>>> http://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/log/?h=perf/core
>>>
>>>
>> Hi Adrian!
>> Thanx for your reminding. Yes, a more checking is added into the 1st
>> branch.
>> And I think we still can join them in two conditions.
>>
>> Do you think this idea is ok to you? If so, I will update my patch base on
>> the latest code.
> Personally I like code that is easy to read, and I don't think
> combining the conditions does that. But you could set a bool
> and use that to decide what to call.
>
How about add a function in target.c named
perf_target__uses_dummy_map(), and this
function take over the all checking to decide which to call. Then we can
use it here like:
if (perf_target__uses_dummy_map(target)):
evlist->cpus = cpu_map__dummy_new();
else:
evlist->cpus = cpu_map__new(target->cpu_list);
>> Yang
>>>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>>>> ---
>>>> tools/perf/util/evlist.c | 8 +++-----
>>>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>>>> index bbc746a..2258eb4 100644
>>>> --- a/tools/perf/util/evlist.c
>>>> +++ b/tools/perf/util/evlist.c
>>>> @@ -819,11 +819,9 @@ int perf_evlist__create_maps(struct perf_evlist
>>>> *evlist, struct target *target)
>>>> if (evlist->threads == NULL)
>>>> return -1;
>>>> - if (target->force_per_cpu)
>>>> - evlist->cpus = cpu_map__new(target->cpu_list);
>>>> - else if (target__has_task(target))
>>>> - evlist->cpus = cpu_map__dummy_new();
>>>> - else if (!target__has_cpu(target) && !target->uses_mmap)
>>>> + if (!target->force_per_cpu &&
>>>> + (target__has_task(target) ||
>>>> + (!target__has_cpu(target) && !target->uses_mmap)))
>>>> evlist->cpus = cpu_map__dummy_new();
>>>> else
>>>> evlist->cpus = cpu_map__new(target->cpu_list);
>>>>
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-11-27 11:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-27 22:23 [PATCH] perf util: Join four conditions in two in perf_evlist__create_maps() Dongsheng Yang
2013-11-27 9:43 ` Adrian Hunter
2013-11-27 22:52 ` Dongsheng Yang
2013-11-27 10:49 ` Adrian Hunter
2013-11-28 0:36 ` Dongsheng Yang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox