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

Add a PriorityQueueUnmanaged type to the standard library #21432

Open
saltzm opened this issue Sep 16, 2024 · 4 comments · May be fixed by #21433
Open

Add a PriorityQueueUnmanaged type to the standard library #21432

saltzm opened this issue Sep 16, 2024 · 4 comments · May be fixed by #21433
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@saltzm
Copy link

saltzm commented Sep 16, 2024

Several Zig data structures in the standard library have "unmanaged" types that require the user to pass an allocator to any function that may allocate. Examples include ArrayListUnmanaged and HashMapUnmanaged. There is however no such type for PriorityQueue.

I'm currently in need of a PriorityQueueUnmanaged, and I made one myself from the existing PriorityQueue code. I'd be happy to contribute it to the standard library if the contribution would be welcome.

The main questions I'd have for an acceptable implementation would be:

  1. Whether the original PriorityQueue type should be implemented in terms of the new PriorityQueueUnmanaged type. ArrayList seems to have separate implementations rather than having the managed version rely on the unmanaged version and I'm not sure why without reading it more closely. My inclination would be to implement PriorityQueue in terms of PriorityQueueUnmanaged but I'm curious whether others see drawbacks to this approach that I'm not seeing.
  2. Whether there should only be a PriorityQueueUnmanaged type, or whether the base type should be PriorityQueueAlignedUnmanaged similar to ArrayList. I have not done it with AlignedUnmanaged locally but given the backing data is just stored in a slice, I think an AlignedUnmanaged type would make sense.
@saltzm saltzm changed the title Add a PriorityQueueUnmanaged type Add a PriorityQueueUnmanaged type to the standard library Sep 16, 2024
@andrewrk andrewrk added standard library This issue involves writing Zig code for the standard library. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. labels Sep 16, 2024
@andrewrk andrewrk added this to the 0.16.0 milestone Sep 16, 2024
@andrewrk
Copy link
Member

Since this is a lesser used API, I think it's OK to be a little more experimental and aggressive with breakage. My suggestion is to modify the existing one to become unmanaged, and drop the "unmanaged" from the name, same as how is currently done for MultiArrayList.

@Rexicon226
Copy link
Contributor

I agree with Andrew.
Should PriorityDequeue also undergo the same change?

@andrewrk
Copy link
Member

Sure

@andrewrk
Copy link
Member

2. whether the base type should be PriorityQueueAlignedUnmanaged similar to ArrayList. I have not done it with AlignedUnmanaged locally but given the backing data is just stored in a slice, I think an AlignedUnmanaged type would make sense.

Let's start with only having the default-aligned version. I think we can get away with that, and it's nicer to avoid it if possible.

saltzm added a commit to saltzm/zig that referenced this issue Sep 17, 2024
Resolves: ziglang#21432.

Removed the allocator field from the PriorityQueue struct and
adapted functions to take an allocator where needed.

Updated tests accordingly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants