From: I Hsin Cheng <richard120310@gmail.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: Bart Van Assche <bvanassche@acm.org>,
akpm@linux-foundation.org, linux-block@vger.kernel.org,
linux-kernel@vger.kernel.org,
"Ching-Chun (Jim) Huang" <jserv@ccns.ncku.edu.tw>
Subject: Re: [PATCH] blk-wbt: Speed up integer square root in rwb_arm_timer
Date: Sat, 30 Mar 2024 16:45:13 +0800 [thread overview]
Message-ID: <CAH5jb=bptDp43mLJ7A3nZuBnDB=V_wjLa3XqCSfsG8sC0OoFyg@mail.gmail.com> (raw)
In-Reply-To: <CAH5jb=btggiO7Op+k2pTS8LyjcZ0tYpvtWJRmQ8sApD-fHyHXg@mail.gmail.com>
The last email didn't follow the plain-text format, I'm sorry for that,
that's why I resend it. Sorry for the bother.
>> Additionally, why to add a second
>> implementation of int_sqrt() instead of replacing the int_sqrt()
>> implementation in lib/math/int_sqrt.c?
I was thinking about adding an alternative option first rather than
replace the whole int_sqrt() function which is used in many other
parts in the Linux kernel.
>> Since int_sqrt() does not use divisions and since int_fastsqrt() uses
>> divisions, can all CPUs supported by the Linux kernel divide numbers as
>> quickly as the CPU mentioned above?
You're right about that. Thanks for pointing out the problem, I'll try to
replace the divisions maybe with another kind of approximation method.
> The claim that it is floor(sqrt(val)) is not true.
> Trivial example:
>
> 1005117225
> sqrt() 31703.58
> int_sqrt() 30703
> int_fastsqrt() 30821
Thanks for pointing out the problem, I only compare my method with int_sqrt()
and plot the result using gnuplot, the result shown that they gave
very very close
answers, but I didn't count the error based on the integer part of
sqrt(), which is
indeed necessary. Sorry for this part. I'll check on the precision of my method
again.
Thanks for your patience and time on reviewing my patch.
Best Regards,
I Hsin Cheng.
On Sat, Mar 30, 2024 at 4:29 PM 鄭以新 <richard120310@gmail.com> wrote:
>
> >> Additionally, why to add a second
> >> implementation of int_sqrt() instead of replacing the int_sqrt()
> >> implementation in lib/math/int_sqrt.c?
>
> I was thinking about adding an alternative option first rather than
> replace the whole int_sqrt() function which is used in many other
> parts in the Linux kernel.
>
> >> Since int_sqrt() does not use divisions and since int_fastsqrt() uses
> >> divisions, can all CPUs supported by the Linux kernel divide numbers as
> >> quickly as the CPU mentioned above?
>
> You're right about that. Thanks for pointing out the problem, I'll try to
> replace the divisions maybe with another kind of approximation method.
>
> > The claim that it is floor(sqrt(val)) is not true.
> > Trivial example:
> >
> > 1005117225
> > sqrt() 31703.58
> > int_sqrt() 30703
> > int_fastsqrt() 30821
>
> Thanks for pointing out the problem, I only compare my method with int_sqrt()
> and plot the result using gnuplot, the result shown that they gave very very close
> answers, but I didn't count the error based on the integer part of sqrt(), which is
> indeed necessary. Sorry for this part. I'll check on the precision of my method
> again.
>
> Thanks for your patience and time on reviewing my patch.
>
> Best Regards,
>
> I Hsin Cheng.
>
> Jens Axboe <axboe@kernel.dk> 於 2024年3月30日 週六 上午3:12寫道：
>>
>> On 3/29/24 12:15 PM, Bart Van Assche wrote:
>> > On 3/29/24 2:12 AM, I Hsin Cheng wrote:
>> >> As the result shown, the origin version of integer square root, which is
>> >> "int_sqrt" takes 35.37 msec task-clock, 1,2181,3348 cycles, 1,6095,3665
>> >> instructions, 2551,2990 branches and causes 1,0616 branch-misses.
>> >>
>> >> At the same time, the variant version of integer square root, which is
>> >> "int_fastsqrt" takes 33.96 msec task-clock, 1,1645,7487 cyclces,
>> >> 5621,0086 instructions, 321,0409 branches and causes 2407 branch-misses.
>> >> We can clearly see that "int_fastsqrt" performs faster and better result
>> >> so it's indeed a faster invariant of integer square root.
>> >
>> > I'm not sure that a 4% performance improvement is sufficient to
>> > replace the int_sqrt() implementation. Additionally, why to add a
>> > second implementation of int_sqrt() instead of replacing the
>> > int_sqrt() implementation in lib/math/int_sqrt.c?
>>
>> That's the real question imho - if provides the same numbers and is
>> faster, why have two?
>>
>> I ran a quick test because I was curious, and the precision is
>> definitely worse. The claim that it is floor(sqrt(val)) is not true.
>> Trivial example:
>>
>> 1005117225
>> sqrt() 31703.58
>> int_sqrt() 30703
>> int_fastsqrt() 30821
>>
>> whether this matters, probably not, but then again it's hard to care
>> about a slow path sqrt calculation. I'd certainly err on the side of
>> precision for that.
>>
>> --
>> Jens Axboe
>>
prev parent reply other threads:[~2024-03-30 8:45 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-29 9:12 [PATCH] blk-wbt: Speed up integer square root in rwb_arm_timer I Hsin Cheng
2024-03-29 18:15 ` Bart Van Assche
2024-03-29 19:12 ` Jens Axboe
[not found] ` <CAH5jb=btggiO7Op+k2pTS8LyjcZ0tYpvtWJRmQ8sApD-fHyHXg@mail.gmail.com>
2024-03-30 8:45 ` I Hsin Cheng [this message]
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='CAH5jb=bptDp43mLJ7A3nZuBnDB=V_wjLa3XqCSfsG8sC0OoFyg@mail.gmail.com' \
--to=richard120310@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=axboe@kernel.dk \
--cc=bvanassche@acm.org \
--cc=jserv@ccns.ncku.edu.tw \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.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.