Skip to content

Commit

Permalink
clarify some code comments
Browse files Browse the repository at this point in the history
  • Loading branch information
abyrd committed Nov 1, 2024
1 parent f65657b commit 5176cba
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 6 deletions.
4 changes: 2 additions & 2 deletions src/main/java/com/conveyal/r5/analyst/NetworkPreloader.java
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ public LoaderState<TransportNetwork> preloadData (AnalysisWorkerTask task) {

/**
* A blocking way to ensure the network and all linkages and precomputed tables are prepared in advance of routing.
* Note that this does not perform any blocking or locking of its own - any synchronization will be that of the
* underlying caches (synchronized methods on TransportNetworkCache or LinkedPointSet). It also bypasses the
* Note that this does not perform any blocking or locking of its own. Any synchronization or turn-taking will be
* that of the underlying caches (TransportNetworkCache or LinkageCache). It also bypasses the
* AsyncLoader locking that would usually allow only one buildValue operation at a time. All threads that call with
* similar tasks will make interleaved calls to setProgress (with superficial map synchronization). Other than
* causing a value to briefly revert from PRESENT to BUILDING this doesn't seem deeply problematic.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -458,10 +458,15 @@ protected void handleOneRegionalTask (RegionalTask task) throws Throwable {
}

// Pull all necessary inputs into cache in a blocking fashion, unlike single-point tasks where prep is async.
// Avoids auto-shutdown while preloading. Must be done after loading destination pointsets to establish extents.
// Note we're completely bypassing the async loader here and relying on the older nested LoadingCaches.
// If those are ever removed, the async loader will need a synchronous mode with per-path blocking (kind of
// reinventing the wheel of LoadingCache) or we'll need to make preparation for regional tasks async.
// This is because single point tasks return fast to report progress, while regional tasks currently do not.
// Worker auto-shutdown time should remain very high during this blocking preload step. Destination point sets
// must already be loaded to establish extents before the preloading step begins. Note that we're still using
// the NetworkPreloader which is an AsyncLoader, but calling a method that intentionally skips all the async or
// background proccessing machinery. Usually, N RegionalTasks all try to preload at once, and all block on
// this method. Redundant slow calculation is not prevented by the AsyncLoader class itself, but by the other
// LoadingCaches behind it. Specifically, the TransportNetworkCache and LinkageCache enforce turn-taking and
// prevent redundant work. If those are ever removed, we would need either async regional task preparation, or
// a synchronous mode with per-key blocking on AsyncLoader (kind of reinventing the wheel of LoadingCache).
TransportNetwork transportNetwork = networkPreloader.synchronousPreload(task);

// If we are generating a static site, there must be a single metadata file for an entire batch of results.
Expand Down

0 comments on commit 5176cba

Please sign in to comment.