[advice] Why not add cmake-format to optional-dependencies ?

[Freed-Wu]

cmake-format is also a python project. Why not add it to optional-dependencies of pyproject.toml? And in the code:

formatted = subprocess.check_output(
    ["cmake-format", "-"],
    cwd=str(Path(doc.path).parent),
    input=content,
    universal_newlines=True,
)

subprocess will fork a thread which is slower than call a python module from python.

If

from cmakelang.format.__main__ import process_file, configuration
process_file(configuration.Configuration(), "set (A  1)\n set (B 3)")

, it will be faster.

[regen100]

Yes, subprocess adds overhead. Still, I initially chose this method because I wanted to avoid adding the formatter as a Python dependent package and creating unnecessary strong coupling. For example, if you have to use multiple versions of a formatter for different projects, subprocess calls make it easy to use multiple versions by tweaking the PATH environment variable. In addition, this LSP already runs cmake with subprocess, and I assumed that the additional overhead of calling the formatter would not be significant.

[Freed-Wu]

avoid adding the formatter as a Python dependent package and creating unnecessary strong coupling.

If we have more formatters, we can add a wrapper to handle the difference to avoid copling. Currently, cmake-format seems to be the only one formatter supported by this project.

tweaking the PATH environment variable.

User can pip install cmake-format==X.Y.Z to use different cmake-format. Or use pyenv, pipenv to install many cmake-formats with different versions in their machine. I think it is not the problem.

overhead of calling the formatter would not be significant.

cmake-format will be called every time when user format. When they write many cmake lang, it will a non-ignoreable overhead.

No offense, in conclusion, I think the performance should be paid more attention.

[regen100]

Dependence on internal functions of external packages must be carefully considered. The internal API tends to be rewritten frequently, and each time I have to maintain this LSP. There is also the disadvantage of forcing the user to use a specific version of the LSP and cmake-format combination.

However, your concern about performance is reasonable. When discussing performance we need to base our discussions on measurements.

I measured the overhead in my environment.

Code:

import subprocess
import timeit
import urllib.request

from cmakelang.format.__main__ import configuration, process_file

urls = [
    "https://raw.githubusercontent.com/Kitware/CMake/master/Utilities/CMakeLists.txt",
    "https://raw.githubusercontent.com/Kitware/CMake/master/CMakeLists.txt",
    "https://raw.githubusercontent.com/Kitware/CMake/master/Source/CMakeLists.txt",
]

for url in urls:
    content = urllib.request.urlopen(url).read().decode()

    def run_subprocess():
        subprocess.check_output(
            ["cmake-format", "-"],
            input=content,
            universal_newlines=True,
        )

    def run_func():
        process_file(configuration.Configuration(), content)

    t_subprocess = min(timeit.repeat(run_subprocess, repeat=100, number=1))
    t_func = min(timeit.repeat(run_func, repeat=100, number=1))

    lines = content.count("\n")
    print(f"{lines=}")
    print(f"{t_subprocess=:.3f}")
    print(f"{t_func=:.3f}")
    print(f"{t_subprocess-t_func=:.3f}")
    print()

lines=21
t_subprocess=0.052
t_func=0.005
t_subprocess-t_func=0.047

lines=554
t_subprocess=1.399
t_func=1.386
t_subprocess-t_func=0.012

lines=1261
t_subprocess=0.729
t_func=0.697
t_subprocess-t_func=0.032

Result shows that the additional overhead is 12~47ms.

Considering the balance between maintenance costs and overhead, I think this overhead is acceptable.

[Freed-Wu]

@cheshirekow (the maintainer of cmake-format) Will process_file() and configuration be rewritten? Or can you provide a stable API for other package developer to call?

[Freed-Wu]

@cheshirekow (the maintainer of cmake-format) Will process_file() and configuration be rewritten? Or can you provide a stable API for other package developer to call?

It looks like cmake-format has been inactive status according to https://github.com/cheshirekow/cmake_format/issues/314. Can I draw the conclusion that this code of this project will not have any changes so we can use its APIs without the fear that the internal API tends to be rewritten?

[Freed-Wu]

No response, close it temporarily.