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 LoadOrTryCompute method #150

Closed
mattjohnsonpint opened this issue Dec 6, 2024 · 5 comments · Fixed by #153
Closed

Add LoadOrTryCompute method #150

mattjohnsonpint opened this issue Dec 6, 2024 · 5 comments · Fixed by #153
Labels
enhancement New feature or request

Comments

@mattjohnsonpint
Copy link

#103 describes how to raise an error if Compute fails. In that example, return 0, true ensures that the 0 is not added to the map.

That works fine for Compute, but I don't see a similar approach possible for LoadOrCompute - since the valueFn doesn't have a delete return value.

So, when using LoadOrCompute, how can I ensure that if an error occurs that I don't add a zero value (or nil, etc.) to the map? I wouldn't want a concurrent thread to retrieve that and treat it as an actual good value.

TLDR: Can you please add a version of LoadOrCompute that accepts a valueFn func() (newValue V, delete bool)?

Thanks.

@puzpuzpuz puzpuzpuz added the question Further information is requested label Dec 7, 2024
@puzpuzpuz
Copy link
Owner

puzpuzpuz commented Dec 7, 2024

Compute is flexible enough to be used in LoadOrCompute style, so you could use it instead of LoadOrCompute:

counts := xsync.NewIntegerMapOf[int, int]()

v, ok := counts.Compute(42, func(oldValue int, loaded bool) (newValue int, delete bool) {
	if loaded {
		return oldValue, false // return the existing value
	}
	// the error-aware logic goes here
	return
})

Does this make sense?

@puzpuzpuz
Copy link
Owner

Closing this one as it's been as is for a while. Feel free to comment if something is unclear.

@mattjohnsonpint
Copy link
Author

Sorry for the delay. Yes, I can make Compute work, if in the case of an error I return 0, true. However, if I'm interpreting the comments correctly, Compute is taking a full lock with each call. So there's a perf implication. I can reduce this by using Load first, but that feels a lot like double-checked locking, which I'd like to avoid. Isn't that the point of LoadOrCompute?

@puzpuzpuz puzpuzpuz reopened this Jan 23, 2025
@puzpuzpuz puzpuzpuz added enhancement New feature or request and removed question Further information is requested labels Jan 23, 2025
@puzpuzpuz
Copy link
Owner

Ok, I understand the use case now: you want to be able to skip adding a computed value. Sounds like you need a new method like LoadOrComputeIf or LoadOrTryCompute. I'll add it in one of the upcoming versions.

@puzpuzpuz puzpuzpuz changed the title LoadOrCompute error handling Add LoadOrTryCompute method Jan 23, 2025
@mattjohnsonpint
Copy link
Author

LoadOrTryCompute sounds perfect. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants