[4/9] select: Micro-optimise __estimate_accuracy()

Submitted by Jann Horn via Containers on Sept. 9, 2019, 10:23 a.m.

Details

Message ID 20190909102340.8592-5-dima@arista.com
State New
Series "restart_block: Prepare the ground for dumping timeout"
Headers show

Commit Message

Jann Horn via Containers Sept. 9, 2019, 10:23 a.m.
Shift on s64 is faster than division, use it instead.

As the result of the patch there is a hardly user-visible effect:
poll(), select(), etc syscalls will be a bit more precise on ~2.3%
than before because 1000 != 1024 :)

Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 fs/select.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Patch hide | download patch | download mbox

diff --git a/fs/select.c b/fs/select.c
index 12cdefd3be2d..2477c202631e 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -51,15 +51,14 @@ 
 
 static long __estimate_accuracy(ktime_t slack)
 {
-	int divfactor = 1000;
-
 	if (slack < 0)
 		return 0;
 
-	if (task_nice(current) > 0)
-		divfactor = divfactor / 5;
+	/* A bit more precise than 0.1% */
+	slack = slack >> 10;
 
-	slack = ktime_divns(slack, divfactor);
+	if (task_nice(current) > 0)
+		slack = slack * 5;
 
 	if (slack > MAX_SLACK)
 		return MAX_SLACK;

Comments

Cyrill Gorcunov Sept. 9, 2019, 11:18 a.m.
On Mon, Sep 09, 2019 at 11:23:35AM +0100, Dmitry Safonov wrote:
> Shift on s64 is faster than division, use it instead.
> 
> As the result of the patch there is a hardly user-visible effect:
> poll(), select(), etc syscalls will be a bit more precise on ~2.3%
> than before because 1000 != 1024 :)
> 
> Signed-off-by: Dmitry Safonov <dima@arista.com>

> ---
>  fs/select.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/select.c b/fs/select.c
> index 12cdefd3be2d..2477c202631e 100644
> --- a/fs/select.c
> +++ b/fs/select.c
> @@ -51,15 +51,14 @@
>  
>  static long __estimate_accuracy(ktime_t slack)
>  {
> -	int divfactor = 1000;
> -
>  	if (slack < 0)
>  		return 0;
>  
> -	if (task_nice(current) > 0)
> -		divfactor = divfactor / 5;
> +	/* A bit more precise than 0.1% */
> +	slack = slack >> 10;
>  
> -	slack = ktime_divns(slack, divfactor);
> +	if (task_nice(current) > 0)
> +		slack = slack * 5;
>  
>  	if (slack > MAX_SLACK)
>  		return MAX_SLACK;

Compiler precompute constants so it doesn't do division here.
But I didn't read the series yet so I might be missing
something obvious.
Dmitry Safonov Sept. 9, 2019, 11:50 a.m.
Hi Cyrill,

On Mon, 9 Sep 2019 at 12:18, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> Compiler precompute constants so it doesn't do division here.
> But I didn't read the series yet so I might be missing
> something obvious.

Heh, like a division is in ktime_divns()?

Thanks,
             Dmitry
Cyrill Gorcunov Sept. 9, 2019, 12:14 p.m.
On Mon, Sep 09, 2019 at 12:50:27PM +0100, Dmitry Safonov wrote:
> Hi Cyrill,
> 
> On Mon, 9 Sep 2019 at 12:18, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> > Compiler precompute constants so it doesn't do division here.
> > But I didn't read the series yet so I might be missing
> > something obvious.
> 
> Heh, like a division is in ktime_divns()?

Ah, you meant the ktime_divns you've dropped out. I thought
you were talking about the constant value we've had here before
your patch. Seems I didn't got the changelog right. Anyway
need to take more precise look on the series. Hopefully soon.
Cyrill Gorcunov Sept. 19, 2019, 2:05 p.m.
On Mon, Sep 09, 2019 at 11:23:35AM +0100, Dmitry Safonov wrote:
> Shift on s64 is faster than division, use it instead.
> 
> As the result of the patch there is a hardly user-visible effect:
> poll(), select(), etc syscalls will be a bit more precise on ~2.3%
> than before because 1000 != 1024 :)
> 
> Signed-off-by: Dmitry Safonov <dima@arista.com>
> ---
>  fs/select.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/select.c b/fs/select.c
> index 12cdefd3be2d..2477c202631e 100644
> --- a/fs/select.c
> +++ b/fs/select.c
> @@ -51,15 +51,14 @@
>  
>  static long __estimate_accuracy(ktime_t slack)
>  {
> -	int divfactor = 1000;
> -
>  	if (slack < 0)
>  		return 0;

Btw, don't you better use <= here?
Dmitry Safonov Sept. 19, 2019, 2:25 p.m.
On 9/19/19 3:05 PM, Cyrill Gorcunov wrote:
[..]
>> diff --git a/fs/select.c b/fs/select.c
>> index 12cdefd3be2d..2477c202631e 100644
>> --- a/fs/select.c
>> +++ b/fs/select.c
>> @@ -51,15 +51,14 @@
>>  
>>  static long __estimate_accuracy(ktime_t slack)
>>  {
>> -	int divfactor = 1000;
>> -
>>  	if (slack < 0)
>>  		return 0;
> 
> Btw, don't you better use <= here?
> 

Good point, will do for v2.

Thanks,
          Dmitry