Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1302299 > unrolled thread
| Started by | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| First post | 2016-01-06 02:00 +0100 |
| Last post | 2016-01-07 01:10 +0100 |
| Articles | 7 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v3 3/5] perf tools: Fix dynamic sort keys to sort properly Namhyung Kim <namhyung@kernel.org> - 2016-01-06 02:00 +0100
Re: [PATCH v3 3/5] perf tools: Fix dynamic sort keys to sort properly Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-07 00:10 +0100
Re: [PATCH v3 3/5] perf tools: Fix dynamic sort keys to sort properly Namhyung Kim <namhyung@kernel.org> - 2016-01-07 00:30 +0100
Re: [PATCH v3 3/5] perf tools: Fix dynamic sort keys to sort properly Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-07 00:40 +0100
Re: [PATCH v3 3/5] perf tools: Fix dynamic sort keys to sort properly Namhyung Kim <namhyung@kernel.org> - 2016-01-07 00:50 +0100
Re: [PATCH v3 3/5] perf tools: Fix dynamic sort keys to sort properly Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-01-07 01:00 +0100
Re: [PATCH v3 3/5] perf tools: Fix dynamic sort keys to sort properly Namhyung Kim <namhyung@kernel.org> - 2016-01-07 01:10 +0100
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2016-01-06 02:00 +0100 |
| Subject | [PATCH v3 3/5] perf tools: Fix dynamic sort keys to sort properly |
| Message-ID | <qNKCe-299-7@gated-at.bofh.it> |
Currently, the dynamic sort keys compares trace data using memcmp().
But for output sorting, it should check data size and compare by word.
Also it sorted strings in reverse order, fix it.
Before)
$ perf report -F overhead -s prev_pid,next_pid
...
# Overhead prev_pid next_pid
# ........ .......... ..........
#
0.39% 490 0
9.12% 225 0
0.04% 224 0
0.51% 731 189
0.08% 731 3
0.12% 731 0
4.82% 729 0
0.08% 1229 0
0.20% 715 0
4.78% 189 225
...
After)
$ perf report -F overhead -s prev_pid,next_pid
...
# Overhead prev_pid next_pid
# ........ .......... ..........
#
0.43% 0 7
0.04% 0 11
0.04% 0 12
0.08% 0 14
0.04% 0 17
0.08% 0 19
0.04% 0 22
0.04% 0 27
0.04% 0 37
0.04% 0 42
...
Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/sort.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 46 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 9618a64875c0..264d2b630549 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1798,6 +1798,51 @@ static int64_t __sort__hde_cmp(struct perf_hpp_fmt *fmt,
return memcmp(a->raw_data + offset, b->raw_data + offset, size);
}
+static int64_t __sort__hde_sort(struct perf_hpp_fmt *fmt,
+ struct hist_entry *a, struct hist_entry *b)
+{
+ struct hpp_dynamic_entry *hde;
+ struct format_field *field;
+ unsigned offset, size;
+ int64_t *a64, *b64;
+ int32_t *a32, *b32;
+ int16_t *a16, *b16;
+
+ hde = container_of(fmt, struct hpp_dynamic_entry, hpp);
+
+ field = hde->field;
+ if (field->flags & FIELD_IS_DYNAMIC) {
+ unsigned long long dyn;
+
+ pevent_read_number_field(field, a->raw_data, &dyn);
+ offset = dyn & 0xffff;
+ size = (dyn >> 16) & 0xffff;
+ } else {
+ offset = field->offset;
+ size = field->size;
+ }
+
+ if (field->flags & FIELD_IS_STRING)
+ return strcmp(b->raw_data + offset, a->raw_data + offset);
+
+ switch (size) {
+ case 8:
+ a64 = a->raw_data + offset;
+ b64 = b->raw_data + offset;
+ return *b64 - *a64;
+ case 4:
+ a32 = a->raw_data + offset;
+ b32 = b->raw_data + offset;
+ return *b32 - *a32;
+ case 2:
+ a16 = a->raw_data + offset;
+ b16 = b->raw_data + offset;
+ return *b16 - *a16;
+ default:
+ return memcmp(b->raw_data + offset, a->raw_data + offset, size);
+ }
+}
+
bool perf_hpp__is_dynamic_entry(struct perf_hpp_fmt *fmt)
{
return fmt->cmp == __sort__hde_cmp;
@@ -1826,7 +1871,7 @@ __alloc_dynamic_entry(struct perf_evsel *evsel, struct format_field *field)
hde->hpp.cmp = __sort__hde_cmp;
hde->hpp.collapse = __sort__hde_cmp;
- hde->hpp.sort = __sort__hde_cmp;
+ hde->hpp.sort = __sort__hde_sort;
INIT_LIST_HEAD(&hde->hpp.list);
INIT_LIST_HEAD(&hde->hpp.sort_list);
--
2.6.4
--
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/
[toc] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2016-01-07 00:10 +0100 |
| Message-ID | <qO5nk-7Ld-15@gated-at.bofh.it> |
| In reply to | #1302299 |
Em Wed, Jan 06, 2016 at 09:54:59AM +0900, Namhyung Kim escreveu:
> Currently, the dynamic sort keys compares trace data using memcmp().
> But for output sorting, it should check data size and compare by word.
> Also it sorted strings in reverse order, fix it.
Can this be broken down in two patches? This is complex code, lets try
to make it as bisectable as possible.
- Arnaldo
>
> Before)
>
> $ perf report -F overhead -s prev_pid,next_pid
> ...
> # Overhead prev_pid next_pid
> # ........ .......... ..........
> #
> 0.39% 490 0
> 9.12% 225 0
> 0.04% 224 0
> 0.51% 731 189
> 0.08% 731 3
> 0.12% 731 0
> 4.82% 729 0
> 0.08% 1229 0
> 0.20% 715 0
> 4.78% 189 225
> ...
>
> After)
>
> $ perf report -F overhead -s prev_pid,next_pid
> ...
> # Overhead prev_pid next_pid
> # ........ .......... ..........
> #
> 0.43% 0 7
> 0.04% 0 11
> 0.04% 0 12
> 0.08% 0 14
> 0.04% 0 17
> 0.08% 0 19
> 0.04% 0 22
> 0.04% 0 27
> 0.04% 0 37
> 0.04% 0 42
> ...
>
> Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/util/sort.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 9618a64875c0..264d2b630549 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -1798,6 +1798,51 @@ static int64_t __sort__hde_cmp(struct perf_hpp_fmt *fmt,
> return memcmp(a->raw_data + offset, b->raw_data + offset, size);
> }
>
> +static int64_t __sort__hde_sort(struct perf_hpp_fmt *fmt,
> + struct hist_entry *a, struct hist_entry *b)
> +{
> + struct hpp_dynamic_entry *hde;
> + struct format_field *field;
> + unsigned offset, size;
> + int64_t *a64, *b64;
> + int32_t *a32, *b32;
> + int16_t *a16, *b16;
> +
> + hde = container_of(fmt, struct hpp_dynamic_entry, hpp);
> +
> + field = hde->field;
> + if (field->flags & FIELD_IS_DYNAMIC) {
> + unsigned long long dyn;
> +
> + pevent_read_number_field(field, a->raw_data, &dyn);
> + offset = dyn & 0xffff;
> + size = (dyn >> 16) & 0xffff;
> + } else {
> + offset = field->offset;
> + size = field->size;
> + }
> +
> + if (field->flags & FIELD_IS_STRING)
> + return strcmp(b->raw_data + offset, a->raw_data + offset);
> +
> + switch (size) {
> + case 8:
> + a64 = a->raw_data + offset;
> + b64 = b->raw_data + offset;
> + return *b64 - *a64;
> + case 4:
> + a32 = a->raw_data + offset;
> + b32 = b->raw_data + offset;
> + return *b32 - *a32;
> + case 2:
> + a16 = a->raw_data + offset;
> + b16 = b->raw_data + offset;
> + return *b16 - *a16;
> + default:
> + return memcmp(b->raw_data + offset, a->raw_data + offset, size);
> + }
> +}
> +
> bool perf_hpp__is_dynamic_entry(struct perf_hpp_fmt *fmt)
> {
> return fmt->cmp == __sort__hde_cmp;
> @@ -1826,7 +1871,7 @@ __alloc_dynamic_entry(struct perf_evsel *evsel, struct format_field *field)
>
> hde->hpp.cmp = __sort__hde_cmp;
> hde->hpp.collapse = __sort__hde_cmp;
> - hde->hpp.sort = __sort__hde_cmp;
> + hde->hpp.sort = __sort__hde_sort;
>
> INIT_LIST_HEAD(&hde->hpp.list);
> INIT_LIST_HEAD(&hde->hpp.sort_list);
> --
> 2.6.4
--
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/
[toc] | [prev] | [next] | [standalone]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2016-01-07 00:30 +0100 |
| Message-ID | <qO5GG-7Vl-3@gated-at.bofh.it> |
| In reply to | #1303124 |
On Wed, Jan 06, 2016 at 08:06:43PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jan 06, 2016 at 09:54:59AM +0900, Namhyung Kim escreveu:
> > Currently, the dynamic sort keys compares trace data using memcmp().
> > But for output sorting, it should check data size and compare by word.
> > Also it sorted strings in reverse order, fix it.
>
> Can this be broken down in two patches? This is complex code, lets try
> to make it as bisectable as possible.
OK, I'll break out the string part then. But I think it doesn't help
much to reduce the complexity.
Thanks,
Namhyung
>
> - Arnaldo
>
> >
> > Before)
> >
> > $ perf report -F overhead -s prev_pid,next_pid
> > ...
> > # Overhead prev_pid next_pid
> > # ........ .......... ..........
> > #
> > 0.39% 490 0
> > 9.12% 225 0
> > 0.04% 224 0
> > 0.51% 731 189
> > 0.08% 731 3
> > 0.12% 731 0
> > 4.82% 729 0
> > 0.08% 1229 0
> > 0.20% 715 0
> > 4.78% 189 225
> > ...
> >
> > After)
> >
> > $ perf report -F overhead -s prev_pid,next_pid
> > ...
> > # Overhead prev_pid next_pid
> > # ........ .......... ..........
> > #
> > 0.43% 0 7
> > 0.04% 0 11
> > 0.04% 0 12
> > 0.08% 0 14
> > 0.04% 0 17
> > 0.08% 0 19
> > 0.04% 0 22
> > 0.04% 0 27
> > 0.04% 0 37
> > 0.04% 0 42
> > ...
> >
> > Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/util/sort.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 46 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> > index 9618a64875c0..264d2b630549 100644
> > --- a/tools/perf/util/sort.c
> > +++ b/tools/perf/util/sort.c
> > @@ -1798,6 +1798,51 @@ static int64_t __sort__hde_cmp(struct perf_hpp_fmt *fmt,
> > return memcmp(a->raw_data + offset, b->raw_data + offset, size);
> > }
> >
> > +static int64_t __sort__hde_sort(struct perf_hpp_fmt *fmt,
> > + struct hist_entry *a, struct hist_entry *b)
> > +{
> > + struct hpp_dynamic_entry *hde;
> > + struct format_field *field;
> > + unsigned offset, size;
> > + int64_t *a64, *b64;
> > + int32_t *a32, *b32;
> > + int16_t *a16, *b16;
> > +
> > + hde = container_of(fmt, struct hpp_dynamic_entry, hpp);
> > +
> > + field = hde->field;
> > + if (field->flags & FIELD_IS_DYNAMIC) {
> > + unsigned long long dyn;
> > +
> > + pevent_read_number_field(field, a->raw_data, &dyn);
> > + offset = dyn & 0xffff;
> > + size = (dyn >> 16) & 0xffff;
> > + } else {
> > + offset = field->offset;
> > + size = field->size;
> > + }
> > +
> > + if (field->flags & FIELD_IS_STRING)
> > + return strcmp(b->raw_data + offset, a->raw_data + offset);
> > +
> > + switch (size) {
> > + case 8:
> > + a64 = a->raw_data + offset;
> > + b64 = b->raw_data + offset;
> > + return *b64 - *a64;
> > + case 4:
> > + a32 = a->raw_data + offset;
> > + b32 = b->raw_data + offset;
> > + return *b32 - *a32;
> > + case 2:
> > + a16 = a->raw_data + offset;
> > + b16 = b->raw_data + offset;
> > + return *b16 - *a16;
> > + default:
> > + return memcmp(b->raw_data + offset, a->raw_data + offset, size);
> > + }
> > +}
> > +
> > bool perf_hpp__is_dynamic_entry(struct perf_hpp_fmt *fmt)
> > {
> > return fmt->cmp == __sort__hde_cmp;
> > @@ -1826,7 +1871,7 @@ __alloc_dynamic_entry(struct perf_evsel *evsel, struct format_field *field)
> >
> > hde->hpp.cmp = __sort__hde_cmp;
> > hde->hpp.collapse = __sort__hde_cmp;
> > - hde->hpp.sort = __sort__hde_cmp;
> > + hde->hpp.sort = __sort__hde_sort;
> >
> > INIT_LIST_HEAD(&hde->hpp.list);
> > INIT_LIST_HEAD(&hde->hpp.sort_list);
> > --
> > 2.6.4
--
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/
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2016-01-07 00:40 +0100 |
| Message-ID | <qO5Ql-7Zi-9@gated-at.bofh.it> |
| In reply to | #1303143 |
Em Thu, Jan 07, 2016 at 08:26:45AM +0900, Namhyung Kim escreveu:
> On Wed, Jan 06, 2016 at 08:06:43PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Jan 06, 2016 at 09:54:59AM +0900, Namhyung Kim escreveu:
> > > Currently, the dynamic sort keys compares trace data using memcmp().
> > > But for output sorting, it should check data size and compare by word.
> > > Also it sorted strings in reverse order, fix it.
> >
> > Can this be broken down in two patches? This is complex code, lets try
> > to make it as bisectable as possible.
>
> OK, I'll break out the string part then. But I think it doesn't help
> much to reduce the complexity.
Well, number of patches is not a problem, everytime I see a "Also lets
do this other thing" I cringe, it is automatic, sorry :-\
For reviewing its soooo much better to see things nicely separated, and
sometimes I like one part but not the other, so I pick one and continue
discussion on the other, etc.
- Arnaldo
> Thanks,
> Namhyung
>
>
> >
> > - Arnaldo
> >
> > >
> > > Before)
> > >
> > > $ perf report -F overhead -s prev_pid,next_pid
> > > ...
> > > # Overhead prev_pid next_pid
> > > # ........ .......... ..........
> > > #
> > > 0.39% 490 0
> > > 9.12% 225 0
> > > 0.04% 224 0
> > > 0.51% 731 189
> > > 0.08% 731 3
> > > 0.12% 731 0
> > > 4.82% 729 0
> > > 0.08% 1229 0
> > > 0.20% 715 0
> > > 4.78% 189 225
> > > ...
> > >
> > > After)
> > >
> > > $ perf report -F overhead -s prev_pid,next_pid
> > > ...
> > > # Overhead prev_pid next_pid
> > > # ........ .......... ..........
> > > #
> > > 0.43% 0 7
> > > 0.04% 0 11
> > > 0.04% 0 12
> > > 0.08% 0 14
> > > 0.04% 0 17
> > > 0.08% 0 19
> > > 0.04% 0 22
> > > 0.04% 0 27
> > > 0.04% 0 37
> > > 0.04% 0 42
> > > ...
> > >
> > > Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > ---
> > > tools/perf/util/sort.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
> > > 1 file changed, 46 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> > > index 9618a64875c0..264d2b630549 100644
> > > --- a/tools/perf/util/sort.c
> > > +++ b/tools/perf/util/sort.c
> > > @@ -1798,6 +1798,51 @@ static int64_t __sort__hde_cmp(struct perf_hpp_fmt *fmt,
> > > return memcmp(a->raw_data + offset, b->raw_data + offset, size);
> > > }
> > >
> > > +static int64_t __sort__hde_sort(struct perf_hpp_fmt *fmt,
> > > + struct hist_entry *a, struct hist_entry *b)
> > > +{
> > > + struct hpp_dynamic_entry *hde;
> > > + struct format_field *field;
> > > + unsigned offset, size;
> > > + int64_t *a64, *b64;
> > > + int32_t *a32, *b32;
> > > + int16_t *a16, *b16;
> > > +
> > > + hde = container_of(fmt, struct hpp_dynamic_entry, hpp);
> > > +
> > > + field = hde->field;
> > > + if (field->flags & FIELD_IS_DYNAMIC) {
> > > + unsigned long long dyn;
> > > +
> > > + pevent_read_number_field(field, a->raw_data, &dyn);
> > > + offset = dyn & 0xffff;
> > > + size = (dyn >> 16) & 0xffff;
> > > + } else {
> > > + offset = field->offset;
> > > + size = field->size;
> > > + }
> > > +
> > > + if (field->flags & FIELD_IS_STRING)
> > > + return strcmp(b->raw_data + offset, a->raw_data + offset);
> > > +
> > > + switch (size) {
> > > + case 8:
> > > + a64 = a->raw_data + offset;
> > > + b64 = b->raw_data + offset;
> > > + return *b64 - *a64;
> > > + case 4:
> > > + a32 = a->raw_data + offset;
> > > + b32 = b->raw_data + offset;
> > > + return *b32 - *a32;
> > > + case 2:
> > > + a16 = a->raw_data + offset;
> > > + b16 = b->raw_data + offset;
> > > + return *b16 - *a16;
> > > + default:
> > > + return memcmp(b->raw_data + offset, a->raw_data + offset, size);
> > > + }
> > > +}
> > > +
> > > bool perf_hpp__is_dynamic_entry(struct perf_hpp_fmt *fmt)
> > > {
> > > return fmt->cmp == __sort__hde_cmp;
> > > @@ -1826,7 +1871,7 @@ __alloc_dynamic_entry(struct perf_evsel *evsel, struct format_field *field)
> > >
> > > hde->hpp.cmp = __sort__hde_cmp;
> > > hde->hpp.collapse = __sort__hde_cmp;
> > > - hde->hpp.sort = __sort__hde_cmp;
> > > + hde->hpp.sort = __sort__hde_sort;
> > >
> > > INIT_LIST_HEAD(&hde->hpp.list);
> > > INIT_LIST_HEAD(&hde->hpp.sort_list);
> > > --
> > > 2.6.4
--
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/
[toc] | [prev] | [next] | [standalone]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2016-01-07 00:50 +0100 |
| Message-ID | <qO601-84d-5@gated-at.bofh.it> |
| In reply to | #1303147 |
On Wed, Jan 06, 2016 at 08:31:49PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jan 07, 2016 at 08:26:45AM +0900, Namhyung Kim escreveu:
> > On Wed, Jan 06, 2016 at 08:06:43PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Wed, Jan 06, 2016 at 09:54:59AM +0900, Namhyung Kim escreveu:
> > > > Currently, the dynamic sort keys compares trace data using memcmp().
> > > > But for output sorting, it should check data size and compare by word.
> > > > Also it sorted strings in reverse order, fix it.
> > >
> > > Can this be broken down in two patches? This is complex code, lets try
> > > to make it as bisectable as possible.
> >
> > OK, I'll break out the string part then. But I think it doesn't help
> > much to reduce the complexity.
>
> Well, number of patches is not a problem, everytime I see a "Also lets
> do this other thing" I cringe, it is automatic, sorry :-\
>
> For reviewing its soooo much better to see things nicely separated, and
> sometimes I like one part but not the other, so I pick one and continue
> discussion on the other, etc.
OK, I understand your concern. I'll try to make it more easier to review.
Thanks,
Namhyung
> > >
> > > >
> > > > Before)
> > > >
> > > > $ perf report -F overhead -s prev_pid,next_pid
> > > > ...
> > > > # Overhead prev_pid next_pid
> > > > # ........ .......... ..........
> > > > #
> > > > 0.39% 490 0
> > > > 9.12% 225 0
> > > > 0.04% 224 0
> > > > 0.51% 731 189
> > > > 0.08% 731 3
> > > > 0.12% 731 0
> > > > 4.82% 729 0
> > > > 0.08% 1229 0
> > > > 0.20% 715 0
> > > > 4.78% 189 225
> > > > ...
> > > >
> > > > After)
> > > >
> > > > $ perf report -F overhead -s prev_pid,next_pid
> > > > ...
> > > > # Overhead prev_pid next_pid
> > > > # ........ .......... ..........
> > > > #
> > > > 0.43% 0 7
> > > > 0.04% 0 11
> > > > 0.04% 0 12
> > > > 0.08% 0 14
> > > > 0.04% 0 17
> > > > 0.08% 0 19
> > > > 0.04% 0 22
> > > > 0.04% 0 27
> > > > 0.04% 0 37
> > > > 0.04% 0 42
> > > > ...
> > > >
> > > > Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > > ---
> > > > tools/perf/util/sort.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
> > > > 1 file changed, 46 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> > > > index 9618a64875c0..264d2b630549 100644
> > > > --- a/tools/perf/util/sort.c
> > > > +++ b/tools/perf/util/sort.c
> > > > @@ -1798,6 +1798,51 @@ static int64_t __sort__hde_cmp(struct perf_hpp_fmt *fmt,
> > > > return memcmp(a->raw_data + offset, b->raw_data + offset, size);
> > > > }
> > > >
> > > > +static int64_t __sort__hde_sort(struct perf_hpp_fmt *fmt,
> > > > + struct hist_entry *a, struct hist_entry *b)
> > > > +{
> > > > + struct hpp_dynamic_entry *hde;
> > > > + struct format_field *field;
> > > > + unsigned offset, size;
> > > > + int64_t *a64, *b64;
> > > > + int32_t *a32, *b32;
> > > > + int16_t *a16, *b16;
> > > > +
> > > > + hde = container_of(fmt, struct hpp_dynamic_entry, hpp);
> > > > +
> > > > + field = hde->field;
> > > > + if (field->flags & FIELD_IS_DYNAMIC) {
> > > > + unsigned long long dyn;
> > > > +
> > > > + pevent_read_number_field(field, a->raw_data, &dyn);
> > > > + offset = dyn & 0xffff;
> > > > + size = (dyn >> 16) & 0xffff;
> > > > + } else {
> > > > + offset = field->offset;
> > > > + size = field->size;
> > > > + }
> > > > +
> > > > + if (field->flags & FIELD_IS_STRING)
> > > > + return strcmp(b->raw_data + offset, a->raw_data + offset);
> > > > +
> > > > + switch (size) {
> > > > + case 8:
> > > > + a64 = a->raw_data + offset;
> > > > + b64 = b->raw_data + offset;
> > > > + return *b64 - *a64;
> > > > + case 4:
> > > > + a32 = a->raw_data + offset;
> > > > + b32 = b->raw_data + offset;
> > > > + return *b32 - *a32;
> > > > + case 2:
> > > > + a16 = a->raw_data + offset;
> > > > + b16 = b->raw_data + offset;
> > > > + return *b16 - *a16;
> > > > + default:
> > > > + return memcmp(b->raw_data + offset, a->raw_data + offset, size);
> > > > + }
> > > > +}
> > > > +
> > > > bool perf_hpp__is_dynamic_entry(struct perf_hpp_fmt *fmt)
> > > > {
> > > > return fmt->cmp == __sort__hde_cmp;
> > > > @@ -1826,7 +1871,7 @@ __alloc_dynamic_entry(struct perf_evsel *evsel, struct format_field *field)
> > > >
> > > > hde->hpp.cmp = __sort__hde_cmp;
> > > > hde->hpp.collapse = __sort__hde_cmp;
> > > > - hde->hpp.sort = __sort__hde_cmp;
> > > > + hde->hpp.sort = __sort__hde_sort;
> > > >
> > > > INIT_LIST_HEAD(&hde->hpp.list);
> > > > INIT_LIST_HEAD(&hde->hpp.sort_list);
> > > > --
> > > > 2.6.4
--
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/
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2016-01-07 01:00 +0100 |
| Message-ID | <qO69I-88W-15@gated-at.bofh.it> |
| In reply to | #1303147 |
Em Wed, Jan 06, 2016 at 08:31:49PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Thu, Jan 07, 2016 at 08:26:45AM +0900, Namhyung Kim escreveu: > > On Wed, Jan 06, 2016 at 08:06:43PM -0300, Arnaldo Carvalho de Melo wrote: > > > Em Wed, Jan 06, 2016 at 09:54:59AM +0900, Namhyung Kim escreveu: > > > > Currently, the dynamic sort keys compares trace data using memcmp(). > > > > But for output sorting, it should check data size and compare by word. > > > > Also it sorted strings in reverse order, fix it. > > > > > > Can this be broken down in two patches? This is complex code, lets try > > > to make it as bisectable as possible. > > > > OK, I'll break out the string part then. But I think it doesn't help > > much to reduce the complexity. > > Well, number of patches is not a problem, everytime I see a "Also lets > do this other thing" I cringe, it is automatic, sorry :-\ > > For reviewing its soooo much better to see things nicely separated, and > sometimes I like one part but not the other, so I pick one and continue > discussion on the other, etc. Ah, please rebase from my latest perf/core, I'm still holding on it since some 'perf test' entries are failing and I want to check first if its due to bugs introduced in this branch... - Arnaldo -- 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/
[toc] | [prev] | [next] | [standalone]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2016-01-07 01:10 +0100 |
| Message-ID | <qO6jr-8sH-63@gated-at.bofh.it> |
| In reply to | #1303155 |
On Wed, Jan 06, 2016 at 08:50:45PM -0300, Arnaldo Carvalho de Melo wrote: > Em Wed, Jan 06, 2016 at 08:31:49PM -0300, Arnaldo Carvalho de Melo escreveu: > > Em Thu, Jan 07, 2016 at 08:26:45AM +0900, Namhyung Kim escreveu: > > > On Wed, Jan 06, 2016 at 08:06:43PM -0300, Arnaldo Carvalho de Melo wrote: > > > > Em Wed, Jan 06, 2016 at 09:54:59AM +0900, Namhyung Kim escreveu: > > > > > Currently, the dynamic sort keys compares trace data using memcmp(). > > > > > But for output sorting, it should check data size and compare by word. > > > > > Also it sorted strings in reverse order, fix it. > > > > > > > > Can this be broken down in two patches? This is complex code, lets try > > > > to make it as bisectable as possible. > > > > > > OK, I'll break out the string part then. But I think it doesn't help > > > much to reduce the complexity. > > > > Well, number of patches is not a problem, everytime I see a "Also lets > > do this other thing" I cringe, it is automatic, sorry :-\ > > > > For reviewing its soooo much better to see things nicely separated, and > > sometimes I like one part but not the other, so I pick one and continue > > discussion on the other, etc. > > Ah, please rebase from my latest perf/core, I'm still holding on it > since some 'perf test' entries are failing and I want to check first if > its due to bugs introduced in this branch... Will do! Thanks, Namhyung -- 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/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web