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

proposal: x/sync/singleflight: add generic version #53427

Open
bradfitz opened this issue Jun 17, 2022 · 19 comments
Open

proposal: x/sync/singleflight: add generic version #53427

bradfitz opened this issue Jun 17, 2022 · 19 comments

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Jun 17, 2022

The singleflight package's Group type uses string keys and interface{} values:

https://pkg.go.dev/golang.org/x/sync/singleflight#Group

I've locally forked it to add generics [K comparable, V any] but I'd like to get it upstream instead.

Proposal: add singleflight.TypedGroup[K, V] or singleflight.GenericGroup[K, V] or singleflight.GroupOf[K, V] behind a go1.18 build tag.

Do we have a convention yet on naming new parallel generic types alongside others?

Related: #47657 (for PoolOf, MapOf). (There was also talk of default type parameters so old code could instantiate types and get the old behavior (in this case [string, any]) and new callers could specify types?)

@gopherbot gopherbot added this to the Proposal milestone Jun 17, 2022
bradfitz added a commit to tailscale/tailscale that referenced this issue Jun 17, 2022
Forked from golang.org/x/sync/singleflight at
the x/sync repo's commit 67f06af15bc961c363a7260195bcd53487529a21

Updates golang/go#53427

Change-Id: Iec2b47b7777940017bb9b3db9bd7d93ba4a2e394
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
bradfitz added a commit to tailscale/tailscale that referenced this issue Jun 17, 2022
Forked from golang.org/x/sync/singleflight at
the x/sync repo's commit 67f06af15bc961c363a7260195bcd53487529a21

Updates golang/go#53427

Change-Id: Iec2b47b7777940017bb9b3db9bd7d93ba4a2e394
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
bradfitz added a commit to tailscale/tailscale that referenced this issue Jun 17, 2022
Forked from golang.org/x/sync/singleflight at
the x/sync repo's commit 67f06af15bc961c363a7260195bcd53487529a21

Updates golang/go#53427

Change-Id: Iec2b47b7777940017bb9b3db9bd7d93ba4a2e394
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
bradfitz added a commit to tailscale/tailscale that referenced this issue Jun 17, 2022
Forked from golang.org/x/sync/singleflight at
the x/sync repo's commit 67f06af15bc961c363a7260195bcd53487529a21

Updates golang/go#53427

Change-Id: Iec2b47b7777940017bb9b3db9bd7d93ba4a2e394
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Jun 17, 2022
@ianlancetaylor
Copy link
Contributor

I don't think we have a convention yet.

@icholy
Copy link

icholy commented Jun 19, 2022

What about x/sync/v2/singleflight ?

twitchyliquid64 pushed a commit to tailscale/tailscale that referenced this issue Jun 21, 2022
Forked from golang.org/x/sync/singleflight at
the x/sync repo's commit 67f06af15bc961c363a7260195bcd53487529a21

Updates golang/go#53427

Change-Id: Iec2b47b7777940017bb9b3db9bd7d93ba4a2e394
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
@catatsuy
Copy link

I tried it before. Thank you.

refs: #52520

@gopherbot
Copy link

Change https://go.dev/cl/425187 mentions this issue: all: switch singleflight to use generic

@lmittmann
Copy link

Is there any update on this proposal?

@ianlancetaylor
Copy link
Contributor

@lmittmann No.

@rsc
Copy link
Contributor

rsc commented Feb 9, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Mar 1, 2023

Maybe it should be x/sync/singleflight/v2.Group[K, V]? Note that even the current v0.2.0 can have a subdirectory package named v2 - it doesn't require tagging the whole repo v2.

@rsc
Copy link
Contributor

rsc commented Mar 1, 2023

This really circles back to #48287, which we didn't resolve.

@lmittmann
Copy link

I know this might sound crazy, but how about simply introducing a breaking change in an experimental package, which is v0, and has "looser compatibility requirements"?

@doggedOwl
Copy link

doggedOwl commented Mar 19, 2023

I agree that in this case and more generally in x/* experimental packages the right thing to do is a breaking change, eitherwise the whole purpose of these packages is mute and might as well put them directly under the standard library.

@rsc
Copy link
Contributor

rsc commented Apr 5, 2023

Holding for #48287.

@rsc
Copy link
Contributor

rsc commented Apr 6, 2023

Placed on hold.
— rsc for the proposal review group

@lmittmann
Copy link

@rsc the new discussion around math/rand/v2 (#60751) seems like there is now a consensus on how to update APIs for generics. Might this clear the way for x/sync/singleflight/v2.Group[K, V]?

@lmittmann
Copy link

Might this clear the way for x/sync/singleflight/v2.Group[K, V]?

@rsc @bradfitz have you had a chance to look into this yet?

@jmhodges
Copy link
Contributor

jmhodges commented Aug 31, 2023

I took a crack at the v2 formulation for this change. I've posted it as a draft because it differs enough from the original proposal that I didn't want to send it out for full review unless I heard from @bradfitz or @rsc that this was an okay way to use the original.

Happy to instead create a new proposal ticket for the v2 version of this if those folks would prefer!

@samber
Copy link

samber commented Mar 4, 2024

I made a simple lib for experiments: https://github.com/samber/go-singleflightx

@costela
Copy link
Contributor

costela commented Mar 25, 2024

@rsc this is marked as "on hold" pending the discussion on #48287, which was closed shortly thereafter, AFAICT without a follow-up.

As @lmittmann mentions, the release of math/rand/v2 seems to indicate this blocker has been removed and the proposal could be put back on the table?

@ianlancetaylor
Copy link
Contributor

Thanks. Moving back to incoming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests