| Message ID | 20210814211713.180533-18-yury.norov@gmail.com |
|---|---|
| State | Accepted |
| Commit | 15325b4f768f2b27b5765489eeab6ec0d6b5e902 |
| Headers | show |
| Series | Resend bitmap patches | expand |
On Sun, Aug 15, 2021 at 12:21 AM Yury Norov <yury.norov@gmail.com> wrote: > > bitmap_list_string() is very ineffective when printing bitmaps with long > ranges of set bits because it calls find_next_bit for each bit in the > bitmap. We can do better by detecting ranges of set bits. > > In my environment, before/after is 943008/31008 ns. I would add a couple of words, maybe in parentheses, to describe what your environment is. ... > + buf = number(++buf, end, rtop - 1, default_dec_spec); ++buf is a bit confusing here. Since you will rewrite the buf value anyway, I would write the parameter as buf + 1. -- With Best Regards, Andy Shevchenko
On Sun, Aug 15, 2021 at 02:09:45PM +0300, Andy Shevchenko wrote: > On Sun, Aug 15, 2021 at 12:21 AM Yury Norov <yury.norov@gmail.com> wrote: > > > > bitmap_list_string() is very ineffective when printing bitmaps with long > > ranges of set bits because it calls find_next_bit for each bit in the > > bitmap. We can do better by detecting ranges of set bits. > > > > In my environment, before/after is 943008/31008 ns. > > I would add a couple of words, maybe in parentheses, to describe what > your environment is. > > ... > > > + buf = number(++buf, end, rtop - 1, default_dec_spec); > > ++buf is a bit confusing here. Since you will rewrite the buf value > anyway, I would write the parameter as buf + 1. Agree, it's sloppy. I'll send the patch by tomorrow.
On Sat 2021-08-14 14:17:13, Yury Norov wrote: > bitmap_list_string() is very ineffective when printing bitmaps with long > ranges of set bits because it calls find_next_bit for each bit in the > bitmap. We can do better by detecting ranges of set bits. > > In my environment, before/after is 943008/31008 ns. > > Signed-off-by: Yury Norov <yury.norov@gmail.com> > Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com> I like the patch. The new code is much easier to follow. Feel free to use: Reviewed-by: Petr Mladek <pmladek@suse.com> Best Regards, Petr
On Thu, Aug 26, 2021 at 04:15:09PM +0200, Petr Mladek wrote: > On Sat 2021-08-14 14:17:13, Yury Norov wrote: > > bitmap_list_string() is very ineffective when printing bitmaps with long > > ranges of set bits because it calls find_next_bit for each bit in the > > bitmap. We can do better by detecting ranges of set bits. > > > > In my environment, before/after is 943008/31008 ns. > > > > Signed-off-by: Yury Norov <yury.norov@gmail.com> > > Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > > I like the patch. The new code is much easier to follow. > Feel free to use: > > Reviewed-by: Petr Mladek <pmladek@suse.com> > > Best Regards, > Petr Thanks Petr! The patch is already in the linux-next. Andrew, Stephen, can you please append Petr's reviewed-by? Thanks, Yury
diff --git a/lib/vsprintf.c b/lib/vsprintf.c index dd006adfe853..29a384eee286 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -1241,20 +1241,13 @@ char *bitmap_list_string(char *buf, char *end, unsigned long *bitmap, struct printf_spec spec, const char *fmt) { int nr_bits = max_t(int, spec.field_width, 0); - /* current bit is 'cur', most recently seen range is [rbot, rtop] */ - int cur, rbot, rtop; bool first = true; + int rbot, rtop; if (check_pointer(&buf, end, bitmap, spec)) return buf; - rbot = cur = find_first_bit(bitmap, nr_bits); - while (cur < nr_bits) { - rtop = cur; - cur = find_next_bit(bitmap, nr_bits, cur + 1); - if (cur < nr_bits && cur <= rtop + 1) - continue; - + for_each_set_bitrange(rbot, rtop, bitmap, nr_bits) { if (!first) { if (buf < end) *buf = ','; @@ -1263,15 +1256,12 @@ char *bitmap_list_string(char *buf, char *end, unsigned long *bitmap, first = false; buf = number(buf, end, rbot, default_dec_spec); - if (rbot < rtop) { - if (buf < end) - *buf = '-'; - buf++; - - buf = number(buf, end, rtop, default_dec_spec); - } + if (rtop == rbot + 1) + continue; - rbot = cur; + if (buf < end) + *buf = '-'; + buf = number(++buf, end, rtop - 1, default_dec_spec); } return buf; }