SAP / commerce-gradle-plugin

Gradle plugins for the full development lifecycle of a SAP Commerce project
Apache License 2.0
33 stars 12 forks source link

Simplify removeUnusedExtensions #30

Closed bechte closed 1 year ago

bechte commented 2 years ago

In https://github.com/SAP/commerce-gradle-plugin/blob/1b6dd3b5c1ccfd3741d0d4c36c9e501318e65202/src/main/java/mpern/sap/commerce/build/HybrisPlugin.java#L174

a task "removeUnusedExtensions" is declared and it uses a FileTreeWalker to remove files and directories. In my local tests, this process always exceeds the memory of the gradle process or it runs for more than 15 minutes, before I actually abort it. 👎

Wouldn't it be easier to compare the content of config/localextensions-generate.xml´ file, generated byant extensionsxmlwith the files found using a simplefind . -type f -name extensioninfo.xml` from the "bin" directory?

This delta should show the extensioninfo.xml files from the extensions that need to be removed, therefore, removing their parent directory with rm -rf <PATH> should do the trick and should run very fast.

What do you think, @mpern ?

mpern commented 2 years ago

Thanks for reporting the issue!

I'm using Gradle / Java to delete the files/folders to be OS-agnostic, but you are definitely right about the performance. Because now that you mention it, I had the same issues when implementing cleanPlatform

Long story short: project.delete is not fast enough, need to either use native Java or OS calls like you mentioned :)

iccaprar commented 1 year ago

How about, instead of cleaning up the unused extensions, we extract from the platform zip only the ones needed by local extensions.xml (and dependencies)? If we could improve the extractions of the platform in that direction, I can help with the implementation.

mpern commented 1 year ago

Thats sounds imteresting!

I guess you would: build the complete dependency tree by parsing all extensioninfo.xml files across all zips (dependency configuration hybrisPlatform), resolve it against localextensions.xml and based on the pruned tree, unpakck only what's necessary.

iccaprar commented 1 year ago

Something along the line:

Since the entire parsing of all the extensioninfo.xml files in the commerce suite zip and intpack zip can take some time, to make the next builds faster, we can save the resulting list of extension information objects in a file in the project cache directory and use that file as long as we do not need a different version of the commerce suite or intpack.

As soon as the version of the commerce suite dependency or of the intpack dependency changes from what we saved in the project cache, we proceed to delete the entire hybris bin/modules and bin/platform folders and start over with the steps above.

Another nice bonus we can have is to unzip some "always needed" extensions (e.g. yempty, yocc, ybackoffice), that are handy to have for ant extgen but we do not want them declared in the localextensions.xml of any project. We can configure these as a list property of our plugin.

What do you think?

bechte commented 1 year ago

I like both, the idea and the proposal. Do we have some dry runs to determine what the actual effect on the build time really is? (Just interested in it)

Besides the decrease of build times it will also safe disk space, which i also like.

You got my vote up 👍

mpern commented 1 year ago

I also like both the idea and the proposal, feel free to go ahead and submit a PR!

A few additional remarks:

hybris {
    sparseBootstrap {
        enabled.set(true)
        alwaysIncluded.set(listOf("yempty", ...))
    }
}

(Disclaimer: I just quickly typed this up. Suggestions for better names, different structure, ... are always welcome)

iccaprar commented 1 year ago

Good, then I will implement this in between next week and we will see how it goes.

iccaprar commented 1 year ago
Unpacking platform in sparse mode
Loaded extensions information from project custom folder in 39 ms
Loaded extensions information from hybrisPlatform dependencies in 2628 ms
Loaded all needed extensions from localextensions.xml in 1 ms
Loaded existing extensions information from project folder in 655 ms
No missing SAP Commerce extensions, nothing to unpack

This is from 2211.2 with intextpack 2211.3, on an M1 Max. With these numbers in mind, I do not believe we need any caching of extension info. I would make the PR as is and we can add caching if needed.

iccaprar commented 1 year ago

This is the run on a Windows machine:

Unpacking platform in sparse mode
Loaded extensions information from project custom folder in 160 ms
Loaded extensions information from hybrisPlatform dependencies in 10539 ms
Loaded all needed extensions from localextensions.xml in 2 ms
Loaded existing extensions information from project folder in 107 ms
Some needed SAP Commerce Suite extensions are missing, copying them

It is slower, but still good enough to not require any extra optimization/caching.

mpern commented 1 year ago

closed in favour of sparseBootstrap (#43)