Pull requests

#1 Declined
Repository
spookylukey spookylukey
Branch
default

Reduce the amount of work needed to actually perform a move

Author
  1. Alex Gaynor
Reviewers
Description

Reduce the amount of work needed to actually perform a move

Comments (9)

  1. Luke Plant repo owner

    A rough-and-ready benchmark this gives about a 25% decrease in the performance of that method. Looking at the implementation of random.sample, it does quite a lot of work, including creating lists and sets etc., at least the stdlib version.

  2. Alex Gaynor author

    Cool, I can't seem to figure out where my checkout with the branch went, do you mind just committing that bit yourself without the "merge" button :)

  3. Luke Plant repo owner

    How about something like the following to get two random numbers and map them so that they don't pick the same index:

    c = len(plan)
    t1_index = random.randrange(0, c)
    t2_index = (t1_index + 1 + random.randrange(0, c - 1)) % c
    
  4. Luke Plant repo owner

    It's a trick that will only work for 2 numbers, AFAICS - after that you wrap around again and can clash.

    Overall, this is about 30% improvement - thanks for the suggestion, even if I didn't use your patch directly.

  5. Luke Plant repo owner

    Yep, that's fine. I don't know the details of how much work it would be, either in CPython or PyPy, but imagine it's going to involve memory allocations, possibly moving things around etc, which is certainly more work than not doing that kind of thing!