Page MenuHomeSoftware Heritage

Add a flyweight copy() to SwhGraphProperties to make it threadsafe

Authored by seirl on Aug 4 2022, 6:06 PM.


Group Reviewers
Maniphest Tasks
T4422: Graph property access is not thread-safe

See T4422 for details.

Diff Detail

Event Timeline

Build is green

Patch application report for D8191 (id=29562)

Rebasing onto 555e3b6d3e...

Current branch diff-target is up to date.
Changes applied before test
commit 66e94bc43d9e3ba4323b71166082c04672bc1388
Author: Antoine Pietri <>
Date:   Thu Aug 4 18:06:10 2022 +0200

    Add a flyweight copy() to SwhGraphProperties to make it threadsafe

See for more details.

seirl requested review of this revision.Aug 4 2022, 6:12 PM
JaredR26 added a subscriber: JaredR26.
JaredR26 added inline comments.

FYI, timestamps and content lengths were already threadsafe for reading for me, or if it wasn't, it didn't cause any issues I found. edgeLabelNames and nodeIdMap were the problems, and I'm guessing any of the other string ones.


I'm not sure it matters, but because this.graph can refer to the underlying graph of this.labelledGraph (Line 71, inside loadLabelledGraphOnly), that would result in double-copying. (When you first wrote the copy function, we hadn't yet found the need to assign the underlying to graph) I'm sure it is low overhead, but would it be better to first copy this.labelledGraph and then assign g.g again if labelledGraph exists?


FYI, this was already threadsafe for reading for me.

This revision is now accepted and ready to land.Aug 4 2022, 6:52 PM