stdlib: Make vertex and edge ids in digraph unique

[lucioleKi]

Fix #9191

[github-actions]

CT Test Results

    2 files     96 suites   1h 8m 40s ⏱️
2 175 tests 2 128 ✅ 47 💤 0 ❌
2 538 runs  2 489 ✅ 49 💤 0 ❌

Results for commit 1727aa8.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

• Complete CT logs (Download Logs)
• HTML Documentation (Download HTML Docs)
• Windows Installer

// Erlang/OTP Github Action Bot

[bjorng]

This doesn't fail if I remove the bug fix. You will need to test the return value.

[bjorng]

This cannot fail because you are specifying the name of the new vertex. You will need to call digraph:add_vertex(G) and check the return value. The graph will also have to contain some vertices created with default names.

[bjorng]

erlang:unique_integer/0 will usually return a negative integer. The documentation says that the integer is greater than or equal to zero.

This way of solving the problem works fine on a 64-bit system. In practice, all integers will always be small integers (fitting in a word).

However, on a 32-bit system, the integers from erlang:unique_integer/0 can start to become bignums that no longer fit in a word. That could degrade performance in a long-running system.

So I think it would be better to not change how the integers are generated in digraph, but instead change digraph_utils:subgraph() to ensure that the $eid and $vid values are copied into the new digraph. One way to implement that would be to add a new function digraph:new(G, Type), which would create a new digraph and inherit the $eid and $vid values from G.

[lucioleKi]

Yes, setting $eid and $vid according to the number of vertices and edges in a subgraph would be another way to fix it. I'll make an internal function to do that.

[jhogberg]

However, on a 32-bit system, the integers from erlang:unique_integer/0 can start to become bignums that no longer fit in a word. That could degrade performance in a long-running system.

Certainly, performance will degrade (slightly) once the line is crossed, but erlang:unique_integer([positive]) will still be much faster than the current dance in ETS:

new_edge_id(G) ->
    NT = G#digraph.ntab,
    [{'$eid', K}] = ets:lookup(NT, '$eid'),
    true = ets:delete(NT, '$eid'),
    true = ets:insert(NT, {'$eid', K+1}),
    ['$e' | K].

We can also store a "baseline" number that is the erlang:unique_integer() when the graph was first created, and make all edge identifiers relative to that, eating the bignum cost only once per edge unless a graph becomes too long-lived.

Another variant is to use the counters module.

So I think it would be better to not change how the integers are generated in digraph, but instead change digraph_utils:subgraph() to ensure that the $eid and $vid values are copied into the new digraph. One way to implement that would be to add a new function digraph:new(G, Type), which would create a new digraph and inherit the $eid and $vid values from G.

Would this be exposed to the user? I would expect digraph:new/2 taking an existing graph to make a copy thereof.

[bjorng]

Certainly, performance will degrade (slightly) once the line is crossed, but erlang:unique_integer([positive]) will still be much faster than the current dance in ETS

Yes, but performance could be degraded for other parts of the system that use erlang:unique_integer/1, because it is a global resource.

[lucioleKi]

I'll make another PR with the new fix. This PR should target maint with a different branch name. Closing it now.