Paths

Table of Contentst

Differential D6676

Move SWHID<->node ID conversion in the Java backend
ClosedPublic
Actions

Authored by seirl on Nov 23 2021, 5:28 PM.

Details

Reviewers

vlorentz

Group Reviewers

Reviewers

Maniphest Tasks

T3740: swh-graph: Translate node IDs on the Java side, not Python side

Commits

rDGRPH0b33cff0d228: Move SWHID<->node ID conversion in the Java backend

Summary

Doing the SWHID <-> node ID conversion in the Python side prevents using
the minimal perfect hash function for SWHID -> node ID. This design is
inherited from when we thought we could have a really thin Java layer
and write all the code in Python, but this turned out to be infeasible
in practice.

Instead, we are instead moving more and more things in the Java layer,
until ultimately the entire Python RPC API might be replaced by a pure
Java implementation.

This commit drops the swh/graph/graph.py file for the same reasons
outlined above (it's infeasible to write graph traversal algorithms in
pure Python), along with its associated tests.

Diff Detail

Repository

rDGRPH Compressed graph representation

Branch

master

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 25199
Build 39374: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 39373: arc lint + arc unit

Event Timeline

seirl created this revision.Nov 23 2021, 5:28 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptNov 23 2021, 5:28 PM

Build has FAILED

Patch application report for D6676 (id=24257)

Rebasing onto c29ee2787e...

Current branch diff-target is up to date.

Changes applied before test

commit 117fda901247d78dc91ab8c897e79d0573a45c3a
Author: Antoine Pietri <antoine.pietri1@gmail.com>
Date:   Tue Nov 23 17:22:00 2021 +0100

    Move SWHID<->node ID conversion in the Java backend
    
    Doing the SWHID <-> node ID conversion in the Python side prevents using
    the minimal perfect hash function for SWHID -> node ID. This design is
    inherited from when we thought we could have a really thin Java layer
    and write all the code in Python, but this turned out to be infeasible
    in practice.
    
    Instead, we are instead moving more and more things in the Java layer,
    until ultimately the entire Python RPC API might be replaced by a pure
    Java implementation.
    
    This commit drops the swh/graph/graph.py file for the same reasons
    outlined above (it's infeasible to write graph traversal algorithms in
    pure Python), along with its associated tests.

Link to build: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/139/
See console output for more information: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/139/console

Harbormaster returned this revision to the author for changes because remote builds failed.Nov 23 2021, 5:28 PM

Harbormaster failed remote builds in B25134: Diff 24257!

seirl added a task: T3740: swh-graph: Translate node IDs on the Java side, not Python side.Nov 23 2021, 5:29 PM

Remove walrus operator

Build is green

Patch application report for D6676 (id=24258)

Rebasing onto c29ee2787e...

Current branch diff-target is up to date.

Changes applied before test

commit aa8a578f4891869acf70fd2d3524d411fea7310e
Author: Antoine Pietri <antoine.pietri1@gmail.com>
Date:   Tue Nov 23 17:22:00 2021 +0100

    Move SWHID<->node ID conversion in the Java backend
    
    Doing the SWHID <-> node ID conversion in the Python side prevents using
    the minimal perfect hash function for SWHID -> node ID. This design is
    inherited from when we thought we could have a really thin Java layer
    and write all the code in Python, but this turned out to be infeasible
    in practice.
    
    Instead, we are instead moving more and more things in the Java layer,
    until ultimately the entire Python RPC API might be replaced by a pure
    Java implementation.
    
    This commit drops the swh/graph/graph.py file for the same reasons
    outlined above (it's infeasible to write graph traversal algorithms in
    pure Python), along with its associated tests.

See https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/140/ for more details.

Harbormaster completed remote builds in B25135: Diff 24258.Nov 23 2021, 5:35 PM

seirl requested review of this revision.Nov 23 2021, 5:35 PM

vlorentz added a subscriber: vlorentz.Nov 26 2021, 1:36 PM

vlorentz added inline comments.

java/src/main/java/org/softwareheritage/graph/Entry.java
158–166	silent fallback on invalid `dst`? why?
177–185	ditto

seirl added inline comments.Nov 26 2021, 1:47 PM

java/src/main/java/org/softwareheritage/graph/Entry.java
158–166	It should be `new SWHID(dst)`. It's very weird that the tests didn't catch that...

Fix src/dst inversion, add regression test

seirl marked 2 inline comments as done.Nov 26 2021, 4:33 PM

Build is green

Patch application report for D6676 (id=24325)

Rebasing onto 32bab89d44...

Current branch diff-target is up to date.

Changes applied before test

commit 0b33cff0d228876964144662d4968d906b32b856
Author: Antoine Pietri <antoine.pietri1@gmail.com>
Date:   Tue Nov 23 17:22:00 2021 +0100

    Move SWHID<->node ID conversion in the Java backend
    
    Doing the SWHID <-> node ID conversion in the Python side prevents using
    the minimal perfect hash function for SWHID -> node ID. This design is
    inherited from when we thought we could have a really thin Java layer
    and write all the code in Python, but this turned out to be infeasible
    in practice.
    
    Instead, we are instead moving more and more things in the Java layer,
    until ultimately the entire Python RPC API might be replaced by a pure
    Java implementation.
    
    This commit drops the swh/graph/graph.py file for the same reasons
    outlined above (it's infeasible to write graph traversal algorithms in
    pure Python), along with its associated tests.

See https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/141/ for more details.

Harbormaster completed remote builds in B25199: Diff 24325.Nov 26 2021, 4:36 PM

vlorentz accepted this revision.Nov 26 2021, 4:55 PM

This revision is now accepted and ready to land.Nov 26 2021, 4:55 PM

Closed by commit rDGRPH0b33cff0d228: Move SWHID<->node ID conversion in the Java backend (authored by seirl). · Explain WhyNov 26 2021, 5:05 PM

This revision was automatically updated to reflect the committed changes.

seirl added a commit: rDGRPH0b33cff0d228: Move SWHID<->node ID conversion in the Java backend.

seirl mentioned this in T3302: Write docstrings for each method in swh/graph/backend.py.Jan 17 2022, 4:09 PM