Generic Type for prque #22156
Generic Type for prque #22156
Conversation
joowon-byun
requested review from
holiman,
karalabe,
rjl493456442 and
zsfelfoldi
as code owners
common/prque/sstack.go
Outdated
| if invert { | ||
| return !result | ||
| } | ||
| return result |
There was a problem hiding this comment.
Might be harder on the eyes, but you could just xor result with invert and return that, sth like return invert ^ result. However, there's no xor operator for booleans, so it would instead be something lke return result != invert
common/prque/prque.go
Outdated
| // New creates a new priority queue. | ||
| func New(setIndex SetIndexCallback) *Prque { | ||
| return &Prque{newSstack(setIndex)} | ||
| func New(invert bool, setIndex SetIndexCallback) *Prque { |
There was a problem hiding this comment.
If instead of New(invert bool, ...) you turned it into two functions: New and NewInverted, then the original signature could remain, and the changes wouldn't touch as much code. Plus it's IMO a bit unelegant when the calling code which uses the 'default' always have to pass in a false as the first argument
|
I'm in general against this PR. My hunch is that is significantly impacts performance. Would have been nice to keep the tests and benchmarks from the upstream repo instead of dropping them and only leaving the code here. |
|
Here are the benchmarks from upstream https://github.com/karalabe/cookiejar/blob/master/collections/prque/prque_test.go |
|
I've opened a PR to pull the tests and benchmarks in too: #22157 Please provide a diff as to how your change behaves on the benchmark vs the old code. |
|
@karalabe As you said, the benchmark shows a big degradation. I will close this PR. |
3 participants
prquecan have generic type. It could be used for[]byte,time.Durationor other types if it is defined inSwap.(PR from klaytn : klaytn/klaytn#840)