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

common/prque: generic priority queue #26286

Closed
wants to merge 2 commits into from

Conversation

karalabe
Copy link
Member

@karalabe karalabe commented Dec 1, 2022

This PR converts common/prque to generics and does the necessary refactors to have the code use the new style.

There are two instances in our code: downloader.queue.expire and trie.Sync where I couldn't properly convert because we add 2 different types of items into the same queue:

  • In the downloader, downloader.queue.expire is a generic method that's called on different queue types, but there's no way to have typed method, limitation of go.
  • In the trie sync it's a switch between the old and new trie representation that causes different things to be added, we can fix that later when we deprecate the old stuff.

@karalabe karalabe added this to the 1.11.0 milestone Dec 1, 2022
Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Submodule [evm-benchmarks](https://github.com/ipsilon/evm-benchmarks) updated 25 files

?

@karalabe
Copy link
Member Author

karalabe commented Dec 1, 2022

Submodule [evm-benchmarks](https://github.com/ipsilon/evm-benchmarks) updated 25 files

?

Whaaaa, maybe I didn't update my submodules to whatever was on master?

@karalabe
Copy link
Member Author

karalabe commented Dec 1, 2022

@holiman Fixed, PTAL

type Prque struct {
cont *sstack
type Prque[V any] struct {
cont *sstack[V]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could have parameters P comparable, V any here. That way, one could use any integer type as key.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LES uses some weird "wrap around" notion for the priorities, which requires subtracting one priority from another. @zsfelfoldi said that that is a temporary hack 2 years ago :P Until that's fixed, I can't make the priority "comparable" only.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be the code to make the priority generic too: #26290, but it changes LES in - a probably silently - breaking way. We need a fix / confirm frmo @zsfelfoldi to make this change.

@fjl fjl changed the title common, core, eth, les, trie: make prque generic common/prque: generic priority queue Dec 1, 2022
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.

4 participants