reduced usage of mutable Settings objects in tests

[firewave]

Currently the Settings objects in the tests are all mutable. This might cause some tests to modify the settings as they also contain run-time information which will be used by subsequent tests using the same object.

We should make sure that they are const if possible. Otherwise we need to make sure the tests will always be run on a a fixed configuration.

[firewave]

This is to gain initial feedback on the approach. I will adjust all test then. I also have other PRs which need to be merged first to avoid conflicts.

[firewave]

This needs many other PRs merge first and we also need to adapt #4785 as a const Settings object still appears to be mutable since a lot of tests fail when I make those static. I wouldn't rule out some const_cast occurrences either.

[firewave]

Turns out they are not mutable. I accidentally made objects static which should not.

[danmar]

you are making various Settings objects static. Why?

[firewave]

you are making various Settings objects static. Why?

So they are not instantiated on each test case but only once. Especially if library loading (or copying) is involved that is consuming unnecessary CPU.

[danmar]

I would not make local objects static just to save a few cpu cycles. Can we please stop with all these micro optimisations that only saves a few cpu cycles. It's more important with readability.

Also these objects will then use some mutexes to make these objects threadsafe that has a penalty also.

[danmar]

Could you show some stats. With static or without static. I wonder if you can test that:

time ./testrunner TestOther

[firewave]

Could you show some stats. With static or without static. I wonder if you can test that:

time ./testrunner TestOther

I will give it a spin. It is quite possible this PR is not showing the gains yet and is even showing a slight regression since I introduced several copies which weren't there before and won't be removed until I have applied all changes. I could add the follow-up changes to this PR as well but I would prefer not to make this even bigger than it already is.

There's also some general change to the way we provide the settings to the common check() wrapper which I haven't done yet which might also improve things and get rid of the need of inline/non-static configurations. It's quite incremental and at the end things will be much cleaner and more explicit.

[firewave]

Doing tests I am getting some inconsistent results which is strange. So I will drop the static for now so this can proceed and will do some profiling again after I am done with all changes.

[danmar]

Doing tests I am getting some inconsistent results which is strange. So I will drop the static for now so this can proceed and will do some profiling again after I am done with all changes.

ok thanks as far as I remember that was my biggest concern. There is now some conflict..