Page MenuHomeSoftware Heritage
Differential D8539

Add random directory sampling policy
Needs ReviewPublic
Actions

Authored by Alphare on Sep 26 2022, 6:52 PM.

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
T4591: swh-scanner - improve tree-comparison capabilities
Summary

This makes use of the new discovery algorithm introduced in
`swh-loader-core`, which should help speed up large (think Linux
kernel or way larger) scans.

Most of the time is spend walking the on-disk directory and hashing,
which is where the new optimizations in `swh-model==6.5.0` should come
in handy. Python is close to its limit in that regard, some future
endeavor should look into setting up SWH for native extensions.

Diff Detail

Repository
rDTSCN Code scanner
Branch
discovery
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 31770
Build 49711: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 49710: arc lint + arc unit

Event Timeline

Alphare created this revision.Sep 26 2022, 6:52 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptSep 26 2022, 6:52 PM
Alphare added a parent revision: D8538: Setup async interface for discovery module.Sep 26 2022, 6:52 PM
swh-public-ci added a comment.Sep 26 2022, 6:53 PM

Build has FAILED

Patch application report for D8539 (id=30793)

Rebasing onto 90534ecae4...

Current branch diff-target is up to date.
Changes applied before test
commit c5714a6aa5f31f503bd188e48cfc59d2271ac60b
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Mon Sep 26 18:45:56 2022 +0200

    Add random directory sampling policy
    
    This makes use of the new discovery algorithm introduced in
    ``swh-loader-core``, which should help speed up large (think Linux
    kernel or way larger) scans.
    
    Most of the time is spend walking the on-disk directory and hashing,
    which is where the new optimizations in ``swh-model==6.5.0`` should come
    in handy. Python is close to its limit in that regard, some future
    endeavor should look into setting up SWH for native extensions.

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

Harbormaster returned this revision to the author for changes because remote builds failed.Sep 26 2022, 6:53 PM
Harbormaster failed remote builds in B31770: Diff 30793!
Alphare updated this revision to Diff 30796.Sep 26 2022, 10:56 PM

Use ABC

swh-public-ci added a comment.Sep 26 2022, 10:58 PM

Build has FAILED

Patch application report for D8539 (id=30796)

Rebasing onto 90534ecae4...

Current branch diff-target is up to date.
Changes applied before test
commit cf90184235c442e42d9901870766e3f93248868b
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Mon Sep 26 18:45:56 2022 +0200

    Add random directory sampling policy
    
    This makes use of the new discovery algorithm introduced in
    ``swh-loader-core``, which should help speed up large (think Linux
    kernel or way larger) scans.
    
    Most of the time is spend walking the on-disk directory and hashing,
    which is where the new optimizations in ``swh-model==6.5.0`` should come
    in handy. Python is close to its limit in that regard, some future
    endeavor should look into setting up SWH for native extensions.

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

Harbormaster returned this revision to the author for changes because remote builds failed.Sep 26 2022, 10:58 PM
Harbormaster failed remote builds in B31773: Diff 30796!
Alphare updated this revision to Diff 30878.Sep 28 2022, 11:00 AM

Add test

swh-public-ci added a comment.Sep 28 2022, 11:01 AM

Build has FAILED

Patch application report for D8539 (id=30878)

Rebasing onto 90534ecae4...

Current branch diff-target is up to date.
Changes applied before test
commit 99f70d6fe730bea88e482ff80effcc27800e7a96
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Mon Sep 26 18:45:56 2022 +0200

    Add random directory sampling policy
    
    This makes use of the new discovery algorithm introduced in
    ``swh-loader-core``, which should help speed up large (think Linux
    kernel or way larger) scans.
    
    Most of the time is spend walking the on-disk directory and hashing,
    which is where the new optimizations in ``swh-model==6.5.0`` should come
    in handy. Python is close to its limit in that regard, some future
    endeavor should look into setting up SWH for native extensions.

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

Harbormaster returned this revision to the author for changes because remote builds failed.Sep 28 2022, 11:01 AM
Harbormaster failed remote builds in B31850: Diff 30878!
Alphare updated this revision to Diff 30916.Sep 28 2022, 6:05 PM

Requirements hack for the CI

swh-public-ci added a comment.Sep 28 2022, 6:08 PM

Build is green

Patch application report for D8539 (id=30916)

Rebasing onto f4f0917769...

First, rewinding head to replay your work on top of it...
Applying: Add random directory sampling policy
Changes applied before test
commit d77225e50a47fd884954bf41e0b762062ef30973
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Mon Sep 26 18:45:56 2022 +0200

    Add random directory sampling policy
    
    This makes use of the new discovery algorithm introduced in
    ``swh-loader-core``, which should help speed up large (think Linux
    kernel or way larger) scans.
    
    Most of the time is spend walking the on-disk directory and hashing,
    which is where the new optimizations in ``swh-model==6.5.0`` should come
    in handy. Python is close to its limit in that regard, some future
    endeavor should look into setting up SWH for native extensions.

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

Harbormaster completed remote builds in B31889: Diff 30916.Sep 28 2022, 6:08 PM
Alphare requested review of this revision.Sep 28 2022, 6:08 PM
ardumont added a subscriber: ardumont.Sep 28 2022, 7:14 PM
ardumont added inline comments.
requirements-swh.txt
4 ↗(On Diff #30916)

I guess it's temporary to make the ci build ok.

Why is the discovery code part implemented in the loader.core and not in the storage code (client side)?
(since it's used both by scanner and loader so far, that would have made sense).

Yes, i've asked it twice ¯\_(ツ)_/¯

[1] D8538#222652

swh/scanner/policy.py
268–269

etc... below

(as far as i remember to .swhid does trigger computation, so might as well reduce that)

Alphare updated this revision to Diff 30929.Sep 28 2022, 8:53 PM

Extract swhid computation

swh-public-ci added a comment.Sep 28 2022, 8:57 PM

Build is green

Patch application report for D8539 (id=30929)

Rebasing onto f4f0917769...

First, rewinding head to replay your work on top of it...
Applying: Add random directory sampling policy
Changes applied before test
commit 00fa0e63578400b21d4df6e6b75c9fc733b57861
Author: Raphaël Gomès <rgomes@octobus.net>
Date:   Mon Sep 26 18:45:56 2022 +0200

    Add random directory sampling policy
    
    This makes use of the new discovery algorithm introduced in
    ``swh-loader-core``, which should help speed up large (think Linux
    kernel or way larger) scans.
    
    Most of the time is spend walking the on-disk directory and hashing,
    which is where the new optimizations in ``swh-model==6.5.0`` should come
    in handy. Python is close to its limit in that regard, some future
    endeavor should look into setting up SWH for native extensions.

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

Harbormaster completed remote builds in B31901: Diff 30929.Sep 28 2022, 8:57 PM
Alphare added inline comments.Sep 29 2022, 10:29 AM
requirements-swh.txt
4 ↗(On Diff #30916)

I guess it's temporary to make the ci build ok.

Yep

Why is the discovery code part implemented in the loader.core and not in the storage code (client side)?
(since it's used both by scanner and loader so far, that would have made sense).

I feel like these pieces of code are are implementation details of the archive consumer and not something the storage (even client-side) should know about. I don't feel super strongly about it, it's just my intuition about where the code should live that may make more sense as it grows.

swh/scanner/policy.py
268–269

Ah, good point. I sometimes forget there isn't an optimizer going over my code, heh.

ardumont mentioned this in D8538: Setup async interface for discovery module.Sep 29 2022, 10:40 AM
ardumont added inline comments.Sep 29 2022, 10:43 AM
requirements-swh.txt
4 ↗(On Diff #30929)

If that commit is landed, we can release it so you can have the standard version number requirement here.

4 ↗(On Diff #30916)

ok.

It feels strange to me to have the scanner consumer specifically depend on the loader.core.
Then again scanner does not seem to depend on the storage either currently so ¯\_(ツ)_/¯.

Maybe that means we need to push that code in swh.core which is the sole common part
between all swh components.

But same as you, i won't be too pushy.

ardumont added 1 blocking reviewer(s): Reviewers.Sep 29 2022, 10:44 AM
ardumont accepted this revision as: ardumont.Sep 29 2022, 10:47 AM
ardumont added subscribers: vlorentz, douardda, anlambert, olasd.

Fine, i've one comment i'd like others to have a look at though [1] regarding where
that new discovery (interface) code should go. It feels currently a bit off to me that this code
is in loader-core. Loaders are not the sole archive consumers (scanner, webapp, cli, indexer, cooker, ...).

[1] https://forge.softwareheritage.org/D8539?id=30916#inline-60774

@vlorentz @douardda @olasd @anlambert ^

olasd added a comment.Sep 29 2022, 4:03 PM
In D8539#222800, @ardumont wrote:

Fine, i've one comment i'd like others to have a look at though [1] regarding where
that new discovery (interface) code should go. It feels currently a bit off to me that this code
is in loader-core. Loaders are not the sole archive consumers (scanner, webapp, cli, indexer, cooker, ...).

[1] https://forge.softwareheritage.org/D8539?id=30916#inline-60774

@vlorentz @douardda @olasd @anlambert ^

If it's to be used generically, this discovery code should pretty definitely not be in swh.loader.core.

  • The generic discovery algorithm, and base abstract classes/protocols, should probably be in swh.model, as they're tied to that structure;
  • The swh.storage-based discovery mechanism could live in swh.storage.algorithms, and be used by swh.loader.core;
  • The REST API-based discovery mechanism could live in swh.web.client, or stay in swh.scanner.
bchauvet added a task: T4591: swh-scanner - improve tree-comparison capabilities.Sep 30 2022, 9:40 AM
ardumont mentioned this in D8760: Add payloads to ExtIDs.Oct 31 2022, 2:48 PM
franckbret mentioned this in D8937: Add 'discovery' model.Dec 7 2022, 4:10 PM
franckbret mentioned this in D8938: Add discovery algorithms and RandomDirSamplingDiscoveryGraph.
franckbret added a subscriber: franckbret.Dec 7 2022, 4:28 PM
In D8539#222941, @olasd wrote:
In D8539#222800, @ardumont wrote:

Fine, i've one comment i'd like others to have a look at though [1] regarding where
that new discovery (interface) code should go. It feels currently a bit off to me that this code
is in loader-core. Loaders are not the sole archive consumers (scanner, webapp, cli, indexer, cooker, ...).

[1] https://forge.softwareheritage.org/D8539?id=30916#inline-60774

@vlorentz @douardda @olasd @anlambert ^

If it's to be used generically, this discovery code should pretty definitely not be in swh.loader.core.

  • The generic discovery algorithm, and base abstract classes/protocols, should probably be in swh.model, as they're tied to that structure;
  • The swh.storage-based discovery mechanism could live in swh.storage.algorithms, and be used by swh.loader.core;
  • The REST API-based discovery mechanism could live in swh.web.client, or stay in swh.scanner.

I started to split the code to different modules with #D8937 and #D8938, does it looks good for you?
Also I'm not really sure for the destination of DIscoveryStorageConnection, can it stay in the model or should it go to storage.discovery?

Revision Contents
Changeset List

PathSize
swh/
scanner/
6 lines
84 lines
3 lines

Diff 30793

View Options

swh/scanner/cli.py

Loading...
View Options

swh/scanner/policy.py

Loading...
View Options

swh/scanner/scanner.py

Loading...
About · Developer information · Legal