Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix binary search http://googleresearch.blogspot.de/2006/06/extra-extra-... #1602

Closed
wants to merge 1 commit into from

Conversation

kuebler
Copy link

@kuebler kuebler commented Mar 12, 2014

Fixes calculating the binary search pivot index calculation for large values, c.f. Extra, Extra - Read All About It: Nearly All Binary Searches and Mergesorts are Broken

@kgcrom
Copy link

kgcrom commented Mar 13, 2014

I compare two operation.

test.o:     file format elf64-x86-64


Disassembly of section .text:

0000000000000000 <main>:
#include <stdio.h>

int main() {
   0:   55                      push   rbp
   1:   48 89 e5                mov    rbp,rsp
        int min = 10, max = 16;
   4:   c7 45 f4 0a 00 00 00    mov    DWORD PTR [rbp-0xc],0xa
   b:   c7 45 f8 10 00 00 00    mov    DWORD PTR [rbp-0x8],0x10
        int mid = 0;
  12:   c7 45 fc 00 00 00 00    mov    DWORD PTR [rbp-0x4],0x0
        mid = (min+max) / 2;
  19:   8b 45 f8                mov    eax,DWORD PTR [rbp-0x8]
  1c:   8b 55 f4                mov    edx,DWORD PTR [rbp-0xc]
  1f:   01 d0                   add    eax,edx
  21:   89 c2                   mov    edx,eax
  23:   c1 ea 1f                shr    edx,0x1f
  26:   01 d0                   add    eax,edx
  28:   d1 f8                   sar    eax,1
  2a:   89 45 fc                mov    DWORD PTR [rbp-0x4],eax
        mid = ((unsigned int)min+(unsigned int)max)>>1;
  2d:   8b 55 f4                mov    edx,DWORD PTR [rbp-0xc]
  30:   8b 45 f8                mov    eax,DWORD PTR [rbp-0x8]
  33:   01 d0                   add    eax,edx
  35:   d1 e8                   shr    eax,1
  37:   89 45 fc                mov    DWORD PTR [rbp-0x4],eax
        return 0;
  3a:   b8 00 00 00 00          mov    eax,0x0
}
  3f:   5d                      pop    rbp
  40:   c3                      ret

I think this has the advantage of reducing the operation.
👍

mattsta added a commit to mattsta/redis that referenced this pull request Aug 2, 2014
The classic (min+max)/2 is provably unsafe.  Fixed
as recommended in research:
http://googleresearch.blogspot.com/2006/06/extra-extra-read-all-about-it-nearly.html

Fix inspired by @wjin, but I used a different approach.
(later, I found @kuebler fixed the same issue too).

Fixes redis#1741, redis#1602
@mattsta mattsta mentioned this pull request Aug 2, 2014
mattsta added a commit to mattsta/redis that referenced this pull request Aug 6, 2014
The classic (min+max)/2 is provably unsafe.  Fixed
as recommended in research:
http://googleresearch.blogspot.com/2006/06/extra-extra-read-all-about-it-nearly.html

Fix inspired by @wjin, but I used a different approach.
(later, I found @kuebler fixed the same issue too).

Fixes redis#1741, redis#1602
mattsta added a commit that referenced this pull request Aug 7, 2014
The classic (min+max)/2 is provably unsafe.  Fixed
as recommended in research:
http://googleresearch.blogspot.com/2006/06/extra-extra-read-all-about-it-nearly.html

Fix inspired by @wjin, but I used a different approach.
(later, I found @kuebler fixed the same issue too).

Fixes #1741, #1602
@mattsta
Copy link
Contributor

mattsta commented Aug 25, 2014

Fixed by #1741 and 4aa3300

@mattsta mattsta closed this Aug 25, 2014
mattsta added a commit that referenced this pull request Aug 26, 2014
The classic (min+max)/2 is provably unsafe.  Fixed
as recommended in research:
http://googleresearch.blogspot.com/2006/06/extra-extra-read-all-about-it-nearly.html

Fix inspired by @wjin, but I used a different approach.
(later, I found @kuebler fixed the same issue too).

Fixes #1741, #1602
mattsta added a commit that referenced this pull request Aug 27, 2014
The classic (min+max)/2 is provably unsafe.  Fixed
as recommended in research:
http://googleresearch.blogspot.com/2006/06/extra-extra-read-all-about-it-nearly.html

Fix inspired by @wjin, but I used a different approach.
(later, I found @kuebler fixed the same issue too).

Fixes #1741, #1602
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants