Web store: downloading all assets is suspiciously slow

[pmonks]

When using the Web Store (i.e. with the org.spdx.useJARLicenseInfoOnly JVM property set to false), code that forces Spdx-Java-Library to download all assets for all listed licenses and exceptions is suspiciously slow the first time it runs (i.e. when the local cache is empty). For example on my laptop, on a 1GB residential internet connection, it takes over 3 minutes to complete.

In contrast, on the same laptop and network, the following commands (which approximate what Spdx-Java-Library is doing) demonstrate that the download can be accomplished substantially faster (i.e. in ~3 seconds):

$ time sh -c 'git clone -n --depth=1 --filter=tree:0 https://github.com/spdx/license-list-data; \
              cd license-list-data; \
              git sparse-checkout set --no-cone /text /template; \
              git checkout; \
              rm -rf .git; \
              curl -O https://raw.githubusercontent.com/spdx/license-list-data/refs/heads/main/json/licenses.json; \
              curl -O https://raw.githubusercontent.com/spdx/license-list-data/refs/heads/main/json/exceptions.json' > /dev/null 2>&1

real 0m3.065s
user 0m0.302s
sys  0m0.337s

While it's hard to imagine the JVM equaling the performance of git, the massive difference in runtime (3 minutes vs 3 seconds) raises my suspicions that something is awry in the Web Store code path.

[pmonks]

I haven't checked this in detail, but in looking at a flame graph from a run where all assets are downloaded for all licenses & exceptions, I'm wondering if perhaps the issue is a cache stampede that results in redundant downloads of the same files. From that graph I notice that most downloads seem to be implicitly initiated via org.spdx.library.model.v3_0_1.core.Element.getName(), so for example if 2 different elements for the same listed license are requested concurrently, and the JSON file for that listed license isn't downloaded yet, might 2 downloads be triggered? org.spdx.utility.DownloadCache.getUrlInputStream() doesn't synchronise on URL, but perhaps it should?

The bit that makes me a bit hesitant about this theory is that AFAIK Spdx-Java-Library doesn't internally multi-thread access to element values, and a cache stampede can't occur without concurrent access to the same resource (in this case, elements within the same listed license).

[pmonks]

Ooooh so I reran that "full download" scenario with this implementation of org.spdx.utility.DownloadCache.getUrlInputStream(), and a full download only took 16 seconds! That tends to support the cache stampede theory.

    public InputStream getUrlInputStream(final URL url, final boolean restrictRedirects) throws IOException {
        InputStream result = null;
        if (url != null) {
            if (cacheEnabled) {
                synchronized (url) {
                    result = getUrlInputStreamThroughCache(url, restrictRedirects);
                }
            } else {
                result = getUrlInputStreamDirect(url, restrictRedirects);
            }
        }
        return result;
    }

In general synchronizing on a method argument is not considered a very good Java practice, so I'll see if there's some other way to achieve per-URL synchronization. At least the limitations around synchronization blocks and virtual threads has been eliminated in JVM 25+, so that's no longer a reason to avoid them.

[goneall]

Thanks @pmonks for the analysis and proposed solution.

Interesting - I haven't run into such in extreme performance problem when I've downloaded the assets, but it looks like you found the issue.

No opinion on the synchronization - there are a few alternatives I can think of using a synchronized hash map, but the code would be a bit more involved and messy.

[pmonks]

@goneall I think it's been exacerbated by my internet going wonky over the last few weeks - a blessing in disguise? 😉

And yeah in the end I ditched the approach based on synchronized and switched to using a ConcurrentHashMap combined with ReentrantLocks - that feels more robust to me and avoids some sneaky footguns around synchronising on a caller-provided object. Details in PR #397.