This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author rhettinger
Recipients pitrou, rhettinger, serhiy.storchaka
Date 2015-01-26.03:23:16
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1422242596.81.0.245896021595.issue23269@psf.upfronthosting.co.za>
In-reply-to
Content
> May be worth to move this test outside of the loop as in the 
> set_lookup function.

The loop used to be this way but testing showed no benefit, so a while back I recombined it back to a j=0 start point and the code looked much nicer.  I don't really want to go back if I don't have to.

There may or may not be a microscopic benefit to moving the  leaq 9(%rcx), %rsi / cmpq %rsi, %rax / jae L237 sequence before the table[i+0]->key == NULL check.  I don't still have access to VTune to verify that this highly predictable branch is essentially free.   I do have a preference at this point for the simpler code -- that will make the future optimization work easier (perhaps inlining the lookkey code into set_insert_key, set_discard, and set_contains).  Also, looking at the disassembly of your patch, there is an additional cost for setting up the table[i+1] entry that wasn't there before (two new extra instructions:      leaq    1(%rcx), %rsi; salq $4, %rsi; occur before the normal lookup and test sequence: leaq 0(%rbp,%rsi), %rdx; cmpq $0, (%rdx); je  L237)

That said, if you think it is important to move the table[i+0] test outside the (i + LINEAR_PROBES <= mask) test, just say so and I'll apply your version of the patch instead of mine.  Otherwise, I prefer the cleaner code (both C and generated assembly) of my patch.
History
Date User Action Args
2015-01-26 03:23:16rhettingersetrecipients: + rhettinger, pitrou, serhiy.storchaka
2015-01-26 03:23:16rhettingersetmessageid: <1422242596.81.0.245896021595.issue23269@psf.upfronthosting.co.za>
2015-01-26 03:23:16rhettingerlinkissue23269 messages
2015-01-26 03:23:16rhettingercreate