Skip to content

Add persistent_term:put_new/2 #9681

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

Open
Benjamin-Philip opened this issue Apr 4, 2025 · 6 comments · May be fixed by #9695
Open

Add persistent_term:put_new/2 #9681

Benjamin-Philip opened this issue Apr 4, 2025 · 6 comments · May be fixed by #9695
Assignees
Labels
enhancement team:VM Assigned to OTP team VM

Comments

@Benjamin-Philip
Copy link
Contributor

Benjamin-Philip commented Apr 4, 2025

Is your feature request related to a problem? Please describe.

I'm using persistent_term to store some atomic counters, which are read and updated by multiple processes concurrently. I'm also creating atomic counters for keys that do not already have one concurrently. Once created, the atomic reference itself (i.e. the term that is persisted) is never updated.

However due to the "overwrite if it already exists" nature of persistent_term:put, I cannot persist these terms concurrently as I cannot atomically check if the term doesn't exist and then conditionally put the term.

Describe the solution you'd like

I propose that a persistent_term:put_new/2 function is added that puts a term only if doesn't already exist, returning an error or an atom otherwise.

Describe alternatives you've considered

Presently I sequentialize all writes to the persistent_term from these process with a gen_server, which has a noticeable performance cost.

Another alternative for my problem is to use an agent instead. This has significant performance costs and involves not using persistent_term all together, so I wouldn't call it a real alternative.

Additional context

This already exists with Elixir's Map as Map.put_new, though there they just return the original map and do not return any error on a pre-existing key.

@jhogberg jhogberg self-assigned this Apr 4, 2025
@jhogberg jhogberg added the team:VM Assigned to OTP team VM label Apr 4, 2025
@Benjamin-Philip
Copy link
Contributor Author

@jhogberg, I don't mind submitting a patch for this myself if that means it will be included in the OTP 28 release.

@jhogberg
Copy link
Contributor

jhogberg commented Apr 7, 2025

I'm afraid it's too late for OTP 28. I'm experimenting with a refactor of persistent_term for 29 however, and will see about adding this while I'm at it. :-)

@RoadRunnr
Copy link
Contributor

@Benjamin-Philip I don't think this is needed. You don't need to serialize every access to the persisted terms through a gen_server. Only reads that find that the key does not exists and that want the key to be created, need to be serialized. That would apply to only a few of the first reads.

Even if multiple readers find that key does not exits, they would call the gen_server, which would check again that the key exists and create it if not. That way only the first call to the gen_server would create the key, the others would simply return the now existing value.

This should work nicely:

get_term(Key) ->
    case persistent_term:get(Key, undefined) of
        undefined ->
            gen_server:call(?SERVER, {get_term, Key});
        Value ->
            Value
    end.

...

handle_call({get_term, Key}, From, State) ->
    Ref = case persistent_term:get(Key, undefined) of
              undefined ->
                  R = atomics:new(?Arity, ?Opts),
                  ok = persistent_term:put(Key, R),
                  R;
              Value ->
                  Value
          end,
    {reply, Ref, State};

@Benjamin-Philip
Copy link
Contributor Author

@RoadRunnr, that is what I am effectively doing now.

@Benjamin-Philip
Copy link
Contributor Author

Benjamin-Philip commented Apr 7, 2025

I'm afraid it's too late for OTP 28. I'm experimenting with a refactor of persistent_term for 29 however, and will see about adding this while I'm at it. :-)

I see. I worked on a patch during the weekend and it's almost PR ready... Could it be merged to master even if it isn't included in the OTP 28 release? Or would you prefer that I discard it and leave implementing this to you after the refactor?

@jhogberg
Copy link
Contributor

jhogberg commented Apr 7, 2025

Could it be merged to master even if it isn't included in the OTP 28 release?

No, anything merged to master before the release will be included in 28.

Or would you prefer that I discard it and leave implementing this to you after the refactor?

The refactor is almost done but I haven't decided on including it yet, there are some trade-offs with my new approach that I need to square first. If yours is almost ready, feel free to open a draft PR with your code in whichever state it is, and we'll revisit it if I decide not to refactor. :-)

@Benjamin-Philip Benjamin-Philip linked a pull request Apr 7, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement team:VM Assigned to OTP team VM
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants