Improve dependency file generation

[danrosen25]

Description

Fixes issue generating fortran file dependencies on case-insensitive file systems.

In the existing code the os.path.isfile(filename) will return true when filename doesn't match the case on the file system. This causes a problem later on because the dependency in the .d file doesn't match the case on the file system.

    check_files = [desired_mod+'.F90', desired_mod+'.f90',
                   desired_mod.lower()+'.F90', desired_mod.lower()+'.f90',
                   desired_mod.upper()+'.F90', desired_mod.upper()+'.f90']
    for d in get_cli_search_dirs():
        for cf in check_files:
            filename = os.path.join(d, cf)
            if os.path.isfile(filename):
                if contains_module_definition(filename, desired_mod):
                    return cf

The old code also makes up to (number of files) * (each modules in a file) * (each search dirs) * ((files in each search dir) + 4) calls to search for module definitions. The new code reduces this through caching and ordering the search directories. The performance enhancement is needed because Path(search directory).glob("*.[fF]90") is much slower than the previous os.path.isfile call.

Resolves #1814

Change Log

• search for module in most likely directories first
• search for module by matching filename first
• cache f90 files for each searched directory
• cache previously found module locations
• fixes issue generating dependencies on case insensitive file systems

Performance Comparison

Discover: depgen only	./compile	./compile -j 4	./compile -d
Baseline	0:06:13	0:01:44	0:04:01
Modifications(14fb62)	0:05:44	0:01:34	0:00:30
Modifications(ce52e4)	0:05:33	0:01:29	0:00:29

Discover: compile	./compile	./compile -j 4	./compile -d
Baseline	0:33:42	0:09:02	0:30:41
Modifications(14fb62)	0:33:29	0:08:58	0:28:24
Modifications(ce52e4)	0:32:57	0:08:52	0:28:32

Laptop: depgen only	./compile	./compile -j 4	./compile -d
Baseline	NA	NA	NA
Modifications(14fb62)	0:01:50	0:00:46	0:00:06
Modifications(ce52e4)	0:01:44	0:00:43	0:00:06

Laptop: Compile	./compile	./compile -j 4	./compile -d
Baseline	NA	NA	NA
Modifications(14fb62)	0:12:12	0:03:43	0:10:29
Modifications(ce52e4)	0:11:47	0:03:35	0:10:18

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

[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.

[danrosen25]

@emkemp @jvgeiger
I don't know if I was ever clear on this but ./compile and ./compile -d did not work on case-insensitive file systems. These changes fix that but without slowing down the performance of dependency generation. They actually speed up ./compile -d significantly through caching, which means less calls to the file system.

I started over in 14fb62a and I built a file search cache and a module search cache system. There's also a directory search ordering system. The first time through it will search files matching the module name in the core directory files followed by each parent directory of the current file being processed, then all other directories. Then it will search files not matching the module name in the core directory files followed by each parent directory of the current file being processed, then all other directories.

There's no performance advantage until you start caching, which happens with ./compile -d. The changes do support case-insensitive file systems. I might play around with the searching order to see if there's better performance.

Let me know your thoughts and I can move this from draft to ready.

Dependency generation time and compile times noted in details.

[danrosen25]

@emkemp @jvgeiger
I played around with sorting directories and there's not much of an impact. The quickest thing to do is to search the core directory, then current directory, then parent directory, then direct descendant directories. First search through files matching the module name, then search through the rest of the files. And maintain the original directory ordering. ce52e4f is ready for review.

And remember that the primary purpose is to support case insensitive file systems but in order to do so I needed to speed up the performance of dependency generation so that it didn't quadruple in time. As a benefit ./compile -d is much faster and all other ./compile calls are slightly faster.