Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upbpo-38938: Adjusting the heapq.merge algorithm for fewer comparisons #17729
Conversation
I made a quick review of general aspects of the PR and left some comments. When I have more time I can do a more in-depth review of the PR. |
This comment has been minimized.
This comment has been minimized.
bedevere-bot
commented
Dec 29, 2019
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
merge_dealloc(mergeobject *mo) | ||
{ | ||
PyObject_GC_UnTrack(mo); | ||
Py_XDECREF(mo->tree); |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I have made the requested changes; please review again. What would be best type for
Or is there some better option? |
This comment has been minimized.
This comment has been minimized.
bedevere-bot
commented
Dec 30, 2019
Thanks for making the requested changes! @pablogsal: please review the changes made to this pull request. |
sweeneyde commentedDec 28, 2019
•
edited by bedevere-bot
This pull request would:
heapq.merge
to be a class whose instances are iterable objects, rather than a generator function.heapq.merge
:heapreplace
on (index, iterator, item) tuples__next__
method of the iterator corresponding to the the item we are replacing. It would then sift that new leaf down into the appropriate part of the heap.heapq.merge
According to my benchmarking, this is at least 2.5 times as fast in almost all cases to the existing implementation, and is especially fast (about 5x as fast as the existing way) with small numbers of iterables. I tested up to 12,000 iterables, with varying degrees of "balance" among the number of items in each. Also noteworthy is that fewer comparisons on average were needed with the proposed new method.
https://bugs.python.org/issue38938