compiler-benchmark icon indicating copy to clipboard operation
compiler-benchmark copied to clipboard

add --path option

Open vlebourl opened this issue 2 years ago • 6 comments

Adds a --path optional command line argument to specify a path in which to look for compilers. Possible usage:

benchmark --path=/usr/bin
benchmark --path=/usr/local/bin:/usr/bin

This allows for the testing of several different builds of the same version of compiler, for instance, clang-13 with or without LTO.

vlebourl avatar Nov 23 '21 13:11 vlebourl

Thanks. What is args.path defaulted to?

nordlow avatar Nov 23 '21 14:11 nordlow

Changed default again as per your last comment suggested. Not providing --path yields the current default behavior. --path should be provided as the environment variable does as it relies on os.pathsep to split the path. eg under Unix:

benchmark --path=/usr/local/bin:/usr/bin

vlebourl avatar Nov 24 '21 07:11 vlebourl

example of a use case: compiling clang in 2 stages or not, both with LTO optimization:

| Lang-uage | Temp-lated | Check Time [us/fn] | Compile Time [us/fn] | Build Time [us/fn] | Run Time [us/fn] | Check RSS [kB/fn] | Build RSS [kB/fn] | Exec Version | Exec Path 
| :-------: | ---------- | :----------------: | :------------------: | :----------------: | :--------------: | :---------------: | :---------------: | :----------: | :-------: 
| C++       | No         |   25.9 (best)      |  148.4 (1.0x)        |  169.8 (1.0x)      |   1730 (best)    | sampling error    |   13.5 (1.1x)     | 14.0.0       | ~/Dependencies/llvm-project/llvm/cmake-build-lto/bin/clang-14 
| C++       | No         |   31.3 (1.2x)      |  215.5 (1.5x)        |  244.7 (1.5x)      |   1984 (1.1x)    |    3.4 (best)     | sampling error    | 14.0.0       | ~/Dependencies/llvm-project/llvm/build/tools/clang/stage2-bins/bin/clang-13 
| C++       | Yes        |   40.7 (1.6x)      |  146.8 (1.0x)        |  176.1 (1.1x)      |   1743 (1.0x)    | sampling error    | sampling error    | 14.0.0       | ~/Dependencies/llvm-project/llvm/cmake-build-lto/bin/clang-14 
| C++       | Yes        |   51.8 (2.0x)      |  217.8 (1.5x)        |  257.9 (1.5x)      |   2089 (1.2x)    | sampling error    | sampling error    | 14.0.0       | ~/Dependencies/llvm-project/llvm/build/tools/clang/stage2-bins/bin/clang-13 

vlebourl avatar Nov 24 '21 08:11 vlebourl

Sorry for not seeing this until now but can you please move the iteration over the paths out from each benchmark_... function into into main function and add a exe_path parameter to each benchmark_... function? That's gonna make the diff much smaller in terms of indentation.

Someting like

for exe_path in args.exe_paths:
    if 'Go' in args.languages:
        if 'Check' in args.operations:
                benchmark_Go(results=results, code_paths=code_paths, args=args, op='Check', templated=False, exe_path=exe_path)
        if 'Build' in args.operations:
                benchmark_Go(results=results, code_paths=code_paths, args=args, op='Build', templated=False, exe_path=exe_path)
    # and similarly for other languages

?

While you're at it can you refactor

        if 'Check' in args.operations:
            benchmark_Swift(results=results, code_paths=code_paths, args=args, op='Check', templated=False)
        if 'Build' in args.operations:
            benchmark_Swift(results=results, code_paths=code_paths, args=args, op='Build', templated=False)

to

        for op in args.operations:
            benchmark_Swift(results=results, code_paths=code_paths, args=args, op=op, templated=False)

and have each benchmark_LANG return None if op is not supported?

nordlow avatar Nov 24 '21 08:11 nordlow

good point, I'll have a look! I also wonder whether using a checksum and os.path.realpath(path) could help avoid testing several times the same exe via symlinks.

vlebourl avatar Nov 24 '21 08:11 vlebourl

good point, I'll have a look!

Thanks!

I also wonder whether using a checksum and os.path.realpath(path) could help avoid testing several times the same exe via symlinks.

Sound good to me!

nordlow avatar Nov 24 '21 08:11 nordlow