LLDB Windows x64 buildbot set lit -j8

[omjavaid]

Summary

LLDB windows x64 buildbot has falky behavior on testsuite. I am setting
number of parallel tests to 8 on this buildbot in hope that it improves
testsuite stability like it did for Linux/Arm.

[stella.stamenova]

buildbot/osuosl/master/config/builders.py

1204

Right now the getLLDBCMakeBuildFactory function will separately set LLVM_LIT_ARGS as well. I think they should be set in just one place to make the code more readable. Perhaps we can remove the LLVM_LIT_ARGS from getLLDBCMakeBuildFactory if they are not used anywhere?

[omjavaid]

buildbot/osuosl/master/config/builders.py

1204

extra flags override those as they appended last to cmake_options. That way we set lit args in case use has not supplied any specific ones in extra_cmake_args.

[stella.stamenova]

buildbot/osuosl/master/config/builders.py

1204

OK, if @gkistanova is OK with this approach also, it's fine by me.

1204

Why -s (succinct)?

[omjavaid]

buildbot/osuosl/master/config/builders.py

1204

its mostly cosmetic to show a progress bar.

[gkistanova]

I think they should be set in just one place to make the code more readable.

I was about to suggest the same.

It might be simpler and more straight forward to revert D116972 and just explicitly specify the needed LLVM_LIT_ARGS in the extra_cmake_args.
If you would like to set -v and -j by default, that could be done by something like this:

defult_lit_args = '-v'
if jobs:
    defult_lit_args += " -j{jobs}".format(jobs=jobs)
CmakeCommand.applyDefaultOptions(cmake_options, [
        ('-DLLVM_LIT_ARGS=', defult_lit_args),
        ])

This is not a blocker for this patch, though.

A separate thing. The patch specifies the both s and v, which kinds of contradict each other, What's the point?

[omjavaid]

In D117363#3245124, @gkistanova wrote:

I think they should be set in just one place to make the code more readable.

I was about to suggest the same.

It might be simpler and more straight forward to revert D116972 and just explicitly specify the needed LLVM_LIT_ARGS in the extra_cmake_args.
If you would like to set -v and -j by default, that could be done by something like this:

defult_lit_args = '-v'
if if jobs:
    defult_lit_args += " -j{jobs}".format(jobs=jobs)
CmakeCommand.applyDefaultOptions(cmake_options, [
        ('-DLLVM_LIT_ARGS=', defult_lit_args),
        ])

This is not a blocker for this patch, though.

A separate thing. The patch specifies the both s and v, which kinds of contradict each other, What's the point?

I conveniently copied lit args from arm lldb bot. AFAIK both giving and sv together shows output as well as a progress bar but I am not too sure.

[stella.stamenova]

In D117363#3245124, @gkistanova wrote:

I think they should be set in just one place to make the code more readable.

I was about to suggest the same.

It might be simpler and more straight forward to revert D116972 and just explicitly specify the needed LLVM_LIT_ARGS in the extra_cmake_args.
If you would like to set -v and -j by default, that could be done by something like this:

defult_lit_args = '-v'
if if jobs:
    defult_lit_args += " -j{jobs}".format(jobs=jobs)
CmakeCommand.applyDefaultOptions(cmake_options, [
        ('-DLLVM_LIT_ARGS=', defult_lit_args),
        ])

What about adding a separate parameter for the jobs for the tests? That way a different level of parallelism could be specified for build and test? That's the route I would have gone.

[JDevlieghere]

In D117363#3245185, @stella.stamenova wrote:

What about adding a separate parameter for the jobs for the tests? That way a different level of parallelism could be specified for build and test? That's the route I would have gone.

+1

[gkistanova]

What about adding a separate parameter for the jobs for the tests? That way a different level of parallelism could be specified for build and test? That's the route I would have gone.

I would guess a particular value depends on available worker hardware resources. So, it could be a property, something like a lit_jobs, which gets rendered to LLVM_LIT_ARGS if defined. Similar how jobs property is defined for workers.

# Windows Server 2016 Intel Xeon(R) Quad 2.30 GHz, 56GB of RAM
create_worker("win-py3-buildbot", properties={'jobs' : 64, 'lit_jobs' : 8}, max_builds=1),

I conveniently copied lit args from arm lldb bot. AFAIK both giving and sv together shows output as well as a progress bar but I am not too sure.

I see. The current LIT implementation ignores -v and executes -s no matter in what order they are specified. So, you need the one or the other, not two together. With automated builds, you may want -v, as it prints more information which could be useful for troubleshooting build issues and test failures.

Anyway, feel free to commit this patch as is. It would solve your immediate problem and everything else we are discussing here could be done as a follow up.

[omjavaid]

I have still setting up the Arm WoA worker which requires a bit of tweaking with visual studio config as well. I have not found a final patch for it and going to commit this one while I work on a follow up.

buildbot/osuosl/master/config/builders.py

# LLDB builders.
     'factory' : LLDBBuilder.getLLDBCMakeBuildFactory(
                     clean=True,
                     python_source_dir=r'"C:\Program Files (x86)\Microsoft Visual Studio\Shared\Python36_64"',
                     target_arch='x64',
                     vs="autodetect",
                     test=True,
                     extra_cmake_args=[
                         '-DLLVM_ENABLE_ASSERTIONS=OFF',
+                        '-DLLVM_LIT_ARGS=-svj 8',
                         '-DLLVM_ENABLE_ZLIB=FALSE',
                         '-DLLDB_ENABLE_PYTHON=TRUE'])},
 
 # LLD builders.
 
     {'name' : "lld-x86_64-win",
     'tags'  : ["lld"],
     'workernames' : ["as-worker-93"],