See T4422 for details.
Details
Details
- Reviewers
JaredR26 - Group Reviewers
Reviewers - Maniphest Tasks
- T4422: Graph property access is not thread-safe
Diff Detail
Diff Detail
- Repository
- rDGRPH Compressed graph representation
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 30628 Build 47890: Phabricator diff pipeline on jenkins Jenkins console · Jenkins Build 47889: arc lint + arc unit
Event Timeline
Comment Actions
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 <antoine.pietri1@gmail.com>
Date: Thu Aug 4 18:06:10 2022 +0200
Add a flyweight copy() to SwhGraphProperties to make it threadsafeSee https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/214/ for more details.
| java/src/main/java/org/softwareheritage/graph/SwhGraphProperties.java | ||
|---|---|---|
| 80–84 | 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. | |
| java/src/main/java/org/softwareheritage/graph/SwhUnidirectionalGraph.java | ||
| 152–153 | 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? | |
| java/src/main/java/org/softwareheritage/graph/maps/NodeTypesMap.java | ||
| 57–61 | FYI, this was already threadsafe for reading for me. | |