Tree shaking for modules

[LKedward]

Experimental changes to implement limited dependency tree-shaking/pruning just for modules (#635)

• Source parsing is updated to detect files that only contain modules (FPM_UNIT_MODULE)
  • If a source contains anything outside of a module, then that source cannot be pruned
  • Pruning is only supported for modules with an end module statement since this is used to detect if anything is outside a module. Modules without an end module statement will always be built and linked.
• New prune_build_targets subroutine to perform tree-shaking/pruning of supported modules
  • Supported modules are pruned by recursively enumerating used module dependencies down from the executables
  • If there are no executables, then module dependencies are enumerated from all sources in the top-level package (so only modules from dependencies are pruned)
  • Submodules are pruned if their parents aren't used anywhere
  • Sources that are not pure modules (FPM_UNIT_SUBPROGRAM) are never pruned

Please test and give feedback; there are possibly more edge cases that I have not considered.

TODO:

• Further testing
• Prune based on root package modules when no executables
• Add --all--no-prune flag to fpm build to skip pruning

[milancurcic]

This is a very exciting update, thank you for working on it. I will test it this week. For the skip pruning option, I suggest a --no-prune flag as it's more specific, and it may be that --all will be more appropriate for some other feature.

[milancurcic]

I played with this locally and everything worked as expected. It's a huge UX upgrade, I love it.

What was especially nice was that to build all stdlib modules (in the case of --no-prune or prior to this PR), you need to pass -fno-range-check if using GFortran to build the hash module. Now, if you don't use the hash module, you don't need to build it and you don't need to pass a flag as a workaround to make it build.

I only skimmed through the code changes, they look fine. I don't have enough knowledge to propose more or better testing for this. So, I'm happy with this PR as is.

Thank you!

[awvwgk]

The initial preparation step for the tree-shaking is quite costly, for a project of mine it took ~1 min before starting the build (8 projects, 135 files, 37k LOC, tblite/tblite). Worse, the build eventually failed because in jacobwilliams/json-fortran a couple of modules were pruned.

[milancurcic]

Thanks @awvwgk.

The initial preparation step for the tree-shaking is quite costly, for a project of mine it took ~1 min before starting the build (8 projects, 135 files, 37k LOC, tblite/tblite).

On my 2014 low-end laptop it took 26 s, which nevertheless is quite long. Granted, this is a quite large project--wIth dependencies it's over 100K LOC. I believe all dependency source files need to be parsed before the build can begin (correct me if I'm wrong). I think this project is a good test-case to use to get pruning to work the way we want it.

How do you suggest improving this? At the very least, we should print a message to the stdout that the pruning in progress may take a while, otherwise the user will think that fpm hung.

A possibility is to develop a heuristic to automatically disable pruning if the size of the root project (e.g. measured in LOC) is large relative to individual dependencies. This seems quite involved to do well and could perhaps be a future GSoC project.

Worse, the build eventually failed because in jacobwilliams/json-fortran a couple of modules were pruned.

I can reproduce this with a minimal project that doesn't use json-fortran, but lists it as a dependency. With pruning enabled it attempts to build json-fortran (which it shouldn't), but then it either prunes out a module that's needed, or it messes up the build sequence. I can't tell yet which part of the introduced logic causes this behavior. I agree this should be resolved before moving forward.

[ivan-pi]

Could the long time be explained by the use of a recursive subroutine? Some profiling may explain where the majority of time is spent, and what is the bottleneck.

[awvwgk]

On my 2014 low-end laptop it took 26 s, which nevertheless is quite long. Granted, this is a quite large project--wIth dependencies it's over 100K LOC. I believe all dependency source files need to be parsed before the build can begin (correct me if I'm wrong). I think this project is a good test-case to use to get pruning to work the way we want it.

Certainly not a large one, maybe medium-sized. Large projects in the Fortran world have several million lines of codes. Another good test project is John's general-purpose-fortran.

Regarding the bottleneck, I recommend to do some profiling. I doubt the source scanner will be a time consuming step, but I spotted a couple of quadratic scaling search loops when looking through the patch.

If it turns out that we cannot reduce the startup time for this feature, we have to defer the time consuming step to runtime and create the build graph dynamically. This strategy is quite successful in meson/CMake generated ninja files, which callback into their parent build system for dynamically determining dependencies, startup time is instantaneous and the dependency resolution never fails.

[LKedward]

Thanks all for testing and the feedback! I didn't test on a large project so that's been useful; I now realise that I missed a redundancy check from the recursive enumeration of modules, which is probably the cause of the poor scaling, so I'll add that in.
Not sure what could be causing the other issue, I'll look into that.

[LKedward]

Adding the missing redundancy check to the recursive exploration has reduced the startup time for large projects. Pruning now adds about 5% to the startup time for tblite compared to the main branch on my machine. Please can you test again for yourselves since I didn't encounter startup times of the same order as you two for tblite (about 10secs on 2014 i7 prior to adding redundancy checks, and now about 2.3secs).

The issues with json-fortran were twofold: first, I wasn't enumerating used modules in non-module sources (which can't be pruned) and; the json sources were being misidentified as non-module sources because of preprocessor lines. I've pushed fixes and tests for these as well.

[milancurcic]

Hmm, with the recent updates, fpm doesn't seem to prune for me. To reproduce:

fpm new a
cd a

Add to fpm.toml:

[dependencies]
tblite = { git="https://github.com/tblite/tblite" }

then run fpm build. I expected fpm to not build tblite and dependencies, but it does.

If I run it with fpm build --no-prune, it builds more stuff, which makes me realize that the first run did prune some stuff.

Then I compared the two and found that with --no-prune, fpm build 253 files, and 81 otherwise.

My question is, if I don't use a dependency at all in my programs and modules, but it's listed in fpm.toml as a dependency, is fpm with pruning expected to build some of that dependency? I didn't think so, but maybe I'm wrong.

[LKedward]

Thanks for checking @milancurcic - I can confirm that this is expected behavior because dftd4 (a dependency of tblite) contains a non-module source file compat.f90 which is classified as such since it contains a subroutine defined outside of a module: generate_wsc. Based on the information that fpm has, we cannot prune this file nor any of the modules it uses, hence the parts that got built in your test.

My question is, if I don't use a dependency at all in my programs and modules, but it's listed in fpm.toml as a dependency, is fpm with pruning expected to build some of that dependency?

So to answer your question directly: yes, if you don't use a dependency via a use statement then that dependency shouldn't be built, except for source files that contain functions/subroutines defined outside of a module and c source files (which we cannot currently prune based on the information that fpm has).

[milancurcic]

@LKedward Ah! I forgot about that. All clear. I will continue testing with a few packages of varying sizes.

[awvwgk]

Thanks for the fix, I can depend on json-fortran again 🎉.

Another edge case for the pruning, this time a smaller project (~95k LOC in 205 files in 5 projects, s-dftd3), well not really smaller. I'm currently not enabling tests for the C-API because we haven't released an fpm version supporting it yet, but if you try to build the C-API tester it will fail due to the bind(c) API export getting pruned without any Fortran users.

Apply this patch on top of the current head:

diff --git a/fpm.toml b/fpm.toml
index 249d6db..265eeff 100644
--- a/fpm.toml
+++ b/fpm.toml
@@ -24,3 +24,8 @@ dependencies.toml-f.git = "https://github.com/toml-f/toml-f"
 [[test]]
 name = "tester"
 source-dir = "test/unit"
+
+[[test]]
+name = "api-tester"
+source-dir = "test/api"
+main = "api-test.c"

Try to run a fresh build for the tests

❯ ../fortran-package-manager/_pull/676/bin/fpm test api-tester
...
api-tester                             failed.
[  97%]Compiling...
/usr/bin/ld: build/gfortran_EA1B3FB6891FF6E5/s-dftd3/test_api_api-test.c.o: in function `show_error':
/home/awvwgk/projects/src/git/s-dftd3/test/api/api-test.c:27: undefined reference to `dftd3_get_error'
...
/usr/bin/ld: /home/awvwgk/projects/src/git/s-dftd3/test/api/api-test.c:178: undefined reference to `dftd3_delete_error'
collect2: error: ld returned 1 exit status
<ERROR> Compilation failed for object " api-tester "
<ERROR>stopping due to failed compilation
STOP 1

Same holds for most of the projects I'm developing, they have an api.f90 file somewhere defining a C-API via bind(c) which is than consumed at least in a unit test, but usually by actual C or Python side users. You can find a similar setup in minpack as well, which is even smaller for testing this behavior.

[LKedward]

Thanks @awvwgk - yes this is another scenario I missed. I think it shouldn't be too difficult to detect bind(c) subroutines, just need to take care to distinguish from bind(c) interfaces.

---

Aside: It seems a shame for a package to effectively loose pruning support because it provides a C API; some possible solutions to this could be to provide the C API as a separate package or to have the C API as optional feature (#609).

[awvwgk]

Aside: It seems a shame for a package to effectively loose pruning support because it provides a C API; some possible solutions to this could be to provide the C API as a separate package or to have the C API as optional feature (#609).

Splitting of bindings into separate projects means they will eventually become stale and out-of-sync, this is not an option for long-term maintainability.

Having the possibility to mark part of the code as optional features would be a solution. For the other build systems in my projects (CMake and meson) the C API is indeed optional and usually disabled when the project is included as subproject and statically linked to avoid symbols leaking from shared libraries declared in the superproject.

[awvwgk]

Startup times look okay now

❯ fpm run --runner time -- build -C ../mctc-tools/tblite
Project is up to date
fpm: Entering directory '/home/awvwgk/projects/src/git/mctc-tools/tblite'
Project is up to date
fpm: Leaving directory '/home/awvwgk/projects/src/git/mctc-tools/tblite'

real	0m0.790s
user	0m0.739s
sys	0m0.043s
❯ time fpm build -C ../mctc-tools/tblite
fpm: Entering directory '/home/awvwgk/projects/src/git/mctc-tools/tblite'
Project is up to date
fpm: Leaving directory '/home/awvwgk/projects/src/git/mctc-tools/tblite'

real	0m0.578s
user	0m0.504s
sys	0m0.066s

[awvwgk]

Thanks a lot for addressing the corner cases. I think this looks good to merge now.

[milancurcic]

I haven't played with it further, but given @awvwgk's review I suggest to go ahead and merge.

[LKedward]

Thanks both for reviewing, I'll now merge.

[zoziha]

@LKedward , I found out, that fpm-0.6.0 doesn't compile correctly
geospace-code/h5fortran (it is compilable in fpm-0.5.0).

char_repeat_read                       failed.
[   5%]Compiling...
/usr/bin/ld: build/gfortran_ECCF645F41985A44/h5fortran/example_char_repeat_read.f90.o: in function `MAIN__':
h5fortran/example/char_repeat_read.f90:22: undefined reference to `__h5fortran_MOD_h5write_scalar'
/usr/bin/ld: h5fortran/example/char_repeat_read.f90:31: undefined reference to `__h5fortran_MOD_h5read_scalar'
/usr/bin/ld: build/gfortran_ECCF645F41985A44/h5fortran/libh5fortran.a(src_interface.f90.o):(.data.rel.ro+0x60): undefined reference to `__h5fortran_MOD_h5read_1d'
/usr/bin/ld: build/gfortran_ECCF645F41985A44/h5fortran/libh5fortran.a(src_interface.f90.o):(.data.rel.ro+0x70): undefined reference to `__h5fortran_MOD_h5read_3d'
/usr/bin/ld: build/gfortran_ECCF645F41985A44/h5fortran/libh5fortran.a(src_interface.f90.o):(.data.rel.ro+0x78): undefined reference to `__h5fortran_MOD_h5read_2d'
/usr/bin/ld: build/gfortran_ECCF645F41985A44/h5fortran/libh5fortran.a(src_interface.f90.o):(.data.rel.ro+0x88): undefined reference to `__h5fortran_MOD_h5read_4d'
/usr/bin/ld: build/gfortran_ECCF645F41985A44/h5fortran/libh5fortran.a(src_interface.f90.o):(.data.rel.ro+0x98): undefined reference to `__h5fortran_MOD_h5read_6d'
/usr/bin/ld: build/gfortran_ECCF645F41985A44/h5fortran/libh5fortran.a(src_interface.f90.o):(.data.rel.ro+0xa0): undefined reference to `__h5fortran_MOD_h5read_scalar'
/usr/bin/ld: build/gfortran_ECCF645F41985A44/h5fortran/libh5fortran.a(src_interface.f90.o):(.data.rel.ro+0xa8): undefined reference to `__h5fortran_MOD_h5write_1d'
/usr/bin/ld: build/gfortran_ECCF645F41985A44/h5fortran/libh5fortran.a(src_interface.f90.o):(.data.rel.ro+0xb0): undefined reference to `__h5fortran_MOD_h5write_3d'
/usr/bin/ld: build/gfortran_ECCF645F41985A44/h5fortran/libh5fortran.a(src_interface.f90.o):(.data.rel.ro+0xb8): undefined reference to `__h5fortran_MOD_h5write_4d'
/usr/bin/ld: build/gfortran_ECCF645F41985A44/h5fortran/libh5fortran.a(src_interface.f90.o):(.data.rel.ro+0xc0): undefined reference to `__h5fortran_MOD_h5write_2d'
/usr/bin/ld: build/gfortran_ECCF645F41985A44/h5fortran/libh5fortran.a(src_interface.f90.o):(.data.rel.ro+0xc8): undefined reference to `__h5fortran_MOD_h5read_7d'
/usr/bin/ld: build/gfortran_ECCF645F41985A44/h5fortran/libh5fortran.a(src_interface.f90.o):(.data.rel.ro+0xd0): undefined reference to `__h5fortran_MOD_h5read_5d'
/usr/bin/ld: build/gfortran_ECCF645F41985A44/h5fortran/libh5fortran.a(src_interface.f90.o):(.data.rel.ro+0xe0): undefined reference to `__h5fortran_MOD_h5write_6d'
/usr/bin/ld: build/gfortran_ECCF645F41985A44/h5fortran/libh5fortran.a(src_interface.f90.o):(.data.rel.ro+0xf0): undefined reference to `__h5fortran_MOD_h5write_scalar'
/usr/bin/ld: build/gfortran_ECCF645F41985A44/h5fortran/libh5fortran.a(src_interface.f90.o):(.data.rel.ro+0x168): undefined reference to `__h5fortran_MOD_h5write_7d'
/usr/bin/ld: build/gfortran_ECCF645F41985A44/h5fortran/libh5fortran.a(src_interface.f90.o):(.data.rel.ro+0x170): undefined reference to `__h5fortran_MOD_h5write_5d'
collect2: error: ld returned 1 exit status
<ERROR> Compilation failed for object " char_repeat_read "
<ERROR>stopping due to failed compilation
STOP 1

Taking h5write_scalar as an example, it seems that the submodule file is removed from the dependency tree.

ENV: Ubuntu, fpm-0.6.0 .

[awvwgk]

@zoziha thanks for checking. Can you open a new issue for visibility?

[LKedward]

Thanks for reporting @zoziha, I'll look into this