Improve dependency file generation

[danrosen25]

Description

Improve dependency file generation speed and portability. Dependency file generation does not currently work from within a Docker container that has bind mounted the LISF source code from a case-insensitive file system #1814. Dependency file generation is slow due to multiple calls to list the contents of repeated directories and inefficient searching through directories. This PR reduces calls to list directory contents and sorts that list based on the module name and file location. These PR changes are particularly efficient if all dependency files are generated in a single call to makedep.py because it only generates a list of f90 files once. Therefore Makefile is also enhanced to determine if dependency files have not been generated yet and it will make a single call to makedep.py. Compile time on Discover is reduced by 4m50s due to a ~77% reduction in dependency file generation time.

Note: Single dependency file generation is often slower because previously the makedep.py file would search for module_name.f90 files without building a list of all f90 files. But the logic did not support case-insensitive file systems. Generating dependency files one at a time is only marginally slower on Discover. It went from ~4m56s to ~5m15s to generate all dependency files one at a time. I have ideas to improve the single dependency file generation but it seems unnecessary.

Improvements to compile time
total build time on Discover before changes: 33m39s
total build time on Discover after changes: 28m49s

Improvements to bulk dependency file generation
baseline make depend: 3m25s
improved make depend: 1m20s

Resolves #1814

Change Log

• Search for all f90 files in dirs list once
• Sort f90 files based on module name and file location
• Generate all dependencies files together if first time compiling
• Fix issue generating dependencies on case-insensitive file systems

Testcase

Tested on Discover using lisf_7.5_intel_2023.2.1_s2s with the following user.cfg settings

VIC.4.1.1: Off
VIC.4.1.2: Off

[danrosen25]

Hi @emkemp
I should have mentioned that I tried this but the problem is the above line, -include $(DEPS) triggers %.d: %.F90 rules before any targets are executed, which builds dependency files one at a time. This is too slow, especially after changing os.path.isfile calls to set(Path(d).glob('*.f90')).

There's an alternative solution using target groups &: but this is only available in GNU Make 4.3 and newer, and Discover uses v4.2.

I also didn't like that the the GNU shell call function replaces newlines with spaces but couldn't find a better way. We do have the option to suppress the output entirely.

[emkemp]

Hi @danrosen25

I see your point, and I reverted my changes.

I wonder if it is worth changing the compile script to check if any .d files exist, and if not, call 'make depend' before calling 'make -f Makefile $njobs'. That would avoid use of that shell function, though it could be kept as a fail-safe.

[danrosen25]

@emkemp That would work. The only caveat I can see is that the logic here specifically checks for any of the .d files listed in $(DEPS). So if there is a foo.d file that exists but it isn't listed in $(DEPS) then this solution is still able to trigger building all .d files at once.

[emkemp]

Hi @danrosen25

I want to discuss this with @jvgeiger before making a final decision on this. But my thinking is that, if no .d files are available, it's probably safest to just do a complete rebuild from scratch.

[danrosen25]

@emkemp
I'm suggesting the following changes to the compile script for POSIX-shell compatibility. Do these work for you? The old changes were causing an error.

[emkemp]

@danrosen25 I like your suggestions, and committed them to the PR.

[jvgeiger]

@danrosen25

Regarding "Generate all dependencies files together if first time compiling", I believe that this is not needed. You can run the compile script with the -d option to generate all dependency files together at once. (Or you can directly run make -C make depend).

Would you please make case folding opt-in instead of default? The makedep.py program accepts a few command line options. The Makefile looks for them via the LIS_MAKEDEP_FLAGS environment variable, which, if not present, defaults to "--verbose status". (Note that the verbose option is currently the only option settable by the user. The other two command line options are explicitly set in the Makefile.)

[danrosen25]

@jvgeiger
If you don't default the behavior to compile with -d (make depend) the first time then it will always be slow because each time makedep.py is called the script crawls through every folder in the dirs list. It's approximately 290 directories for close to 2,000 files, which is an upper bound of 580,000 calls to scan for files (glob.glob) in directories. The make depend only scans each directory once so that's 290 calls to scan files in directories. The only reason it doesn't take hours is because the os.path.isfile is very fast and most of the files are named similar to the module, so it doesn't get to the glob.glob.

The reason I even started this work is because os.path.isfile fails on case insensitive file systems because it finds myfile.F90 when the file is actually MyFile.F90 but then the dependency looks like SomeFile.o: myfile.o but it should be written as SomeFile.o: MyFile.o.

I know it's not a high priority to run on Mac OS systems but I can't even run in a container on a Mac OS system if the volume from the Mac OS system is bind mounted into the container, which is how Dev Containers work. I'm looking to enhance the development portability so that development and testing don't have to happen on a small set of HPCs.

[jvgeiger]

Just to be clear, I like your changes to makedep.py.

Yes, we only support Unix/Linux systems with case-sensitive file systems, but I understand your need to support a case-insensive file system. I ask that you make the case folding opt-in because I cannot control the development of third-party models that we incorporate into LIS.

I like the performance improvements in makedep.py when processing all files at once. I am just questioning the need to change the compile script and Makefile. If you run ./compile -d the first time you compile LIS, then the dependencies will be processed all at once. Then for subsequent recompiles, just run ./compile, which will process only the required files one at a time. Our configure, compile, and Makefile scripts are already messy. I am trying to keep them as clean as possible.

By the way, you can combine the -d and -j options to the compile script, meaning ./compile -d -j 16 works.

[danrosen25]

@jvgeiger Okay, I'll have to make some makedep.py changes to make it more efficient in the single dependency generation case if that's going to be the default. What I'll do when there is a single file is call Path(d).glob('.f90|.F90') for the directory of the current file and search for the module then Path(d).glob('.f90|.F90') for other directories. Path(d).glob is slow but supports case-insensitive file systems. The performance improvements in this PR come from calling glob as few times as possible. The code I'm replacing called os.path.isfile followed by glob.glob for each module search in each directory.

[jvgeiger]

Hello Dan,

I apologize for my confusion. I do not understand your response. Please correct me where I am wrong. To me, your changes to makedep.py do two things: case folding and improved file/directory processing. For the improved file/directory processing to work best, you must process all dependencies at once (not file by file). That is what ./compile -d does. I do not understand why using ./compile -d requires more work to makedep.py.

Is ./compile -d broken in your environment?

[danrosen25]

@jvgeiger
The case folding code is irrelevant, str.casefold() and str.lower() basically do the same thing. I'm using it to sort the list of files found by Path.glob() in a "most likely to find the module in this file" sorting.

./compile -d is much much faster because it finds all the .F90 files one time and ./compile is much slower because Path.glob() finds all the .F90 files once per file (so * 2000). Let's say Path.glob takes about 0.005 seconds * number of directories. That's approximately 1.45 seconds. If you do this once, like is done for ./compile -d, then it takes 1.45 seconds. If you do it 2000 times, like is done for ./compile, then it takes 2900 seconds.

If you're running makedep.py for a single file then you'd want to sort the directories by most likely and run Path.glob() then check the files in that directory then run Path.glob() on the next directory. This reduces the Path.glob calls depending on where it finds the module.

os.path.isfile(), which is what is being replaced because it can't handle case insensitive file systems, takes 0.00005 seconds. Although os.path.isfile was only finding files sometimes and then it would go to .glob afterwards.