Blender Linking Time & GTests #73268

Closed
opened 2020-01-20 16:45:47 +01:00 by Bastien Montagne · 44 comments

Currently, doing a full debug build with ASAN and all GTests has become a real pain, as (on my machine, which does have 16GB RAM and a SSD), it can take over 10 minutes to link the four executables (Blender itself, and the three gtests that are also linking most of Blender's library).

While adding more RAM is always an answer (although not always possible in practice), I think we should stop making such 'need everything' tests as independent binaries.

I can see at least two ways to avoid such thing:

  • Do what we already do a lot: use python API and simple python scripts.
  • Integrate those tests that absolutely require to be written in C as part of Blender build itself (behind the right #ifdef of course).

I would expect that #1 would be enough, maybe with a little bit of #2 in the form of having some part of the RNA/Py API hidden behind the GTEST define, to only actually expose them when needed (if we really need C code for performance and/or low-level access reasons).

Currently, doing a full debug build with ASAN and all GTests has become a real pain, as (on my machine, which does have 16GB RAM and a SSD), it can take over 10 minutes to link the four executables (Blender itself, and the three gtests that are also linking most of Blender's library). While adding more RAM is always an answer (although not always possible in practice), I think we should stop making such 'need everything' tests as independent binaries. I can see at least two ways to avoid such thing: - Do what we already do a lot: use python API and simple python scripts. - Integrate those tests that absolutely require to be written in C as part of Blender build itself (behind the right #ifdef of course). I would expect that #1 would be enough, maybe with a little bit of #2 in the form of having some part of the RNA/Py API hidden behind the GTEST define, to only actually expose them when needed (if we really need C code for performance and/or low-level access reasons).
Author
Owner

Changed status from 'Needs Triage' to: 'Confirmed'

Changed status from 'Needs Triage' to: 'Confirmed'
Author
Owner
Added subscribers: @mont29, @brecht, @Sergey, @ideasman42, @dfelinto, @dr.sybren
Member

Added subscriber: @howardt

Added subscriber: @howardt
Member

I think I'm responsible for starting this, about 6 years ago, with bmesh_core_test.cc. I was ambitious to start unit testing all of the primitives used inside BMesh. Many of those functions are not exposed in the Python API, and probably shouldn't be. The tendrils of those primitives extend so far into other parts of Blender that it seemed very hard to isolate small libraries to link against that would let those bmesh files compile & link.

But I never added any more than the trivial proof-of-concept test case that is in that file, and nobody else stepped up to do it either. And honestly, that code works well and rarely (if ever?) breaks, so I would be OK with just deleting that test.

I don't know about the other two gtests referenced in this bug that link against all of Blender.

I think I'm responsible for starting this, about 6 years ago, with bmesh_core_test.cc. I was ambitious to start unit testing all of the primitives used inside BMesh. Many of those functions are not exposed in the Python API, and probably shouldn't be. The tendrils of those primitives extend so far into other parts of Blender that it seemed very hard to isolate small libraries to link against that would let those bmesh files compile & link. But I never added any more than the trivial proof-of-concept test case that is in that file, and nobody else stepped up to do it either. And honestly, that code works well and rarely (if ever?) breaks, so I would be OK with just deleting that test. I don't know about the other two gtests referenced in this bug that link against all of Blender.

For the USD exporter tests I want to be able to construct a scene and inspect the result of the AbstractHierarchyIterator class. This scene construction is currently done by loading a blend file, which of course requires the majority of Blender to be there. What I think would be a good approach is to have a more 'mockable' structure in Blender. For my purpose, it would already be enough to have a dependency graph that produces objects (with their evaluated data) for the USD exporter to consume. If we could have a 'mock depsgraph' that produces test data without having to link to all of Blender, that would certainly be an improvement.

For integration tests we can use Blender + Python just fine. This would allow me to do a file load, export to USD, import into another scene, and compare the result. This, however, is not a unit test.

For the USD exporter tests I want to be able to construct a scene and inspect the result of the `AbstractHierarchyIterator` class. This scene construction is currently done by loading a blend file, which of course requires the majority of Blender to be there. What I think would be a good approach is to have a more 'mockable' structure in Blender. For my purpose, it would already be enough to have a dependency graph that produces objects (with their evaluated data) for the USD exporter to consume. If we could have a 'mock depsgraph' that produces test data without having to link to all of Blender, that would certainly be an improvement. For integration tests we can use Blender + Python just fine. This would allow me to do a file load, export to USD, import into another scene, and compare the result. This, however, is not a unit test.
Author
Owner

@dr.sybren I think you could add C-required unittests as part of Blender build itself, expose the main test function as a python call, and use python script to do the work? C-part of tests being only actualy built when gtests are enabled, of course.

Note that this is only a very rough random idea, we could do that in many different ways/code layout, how to do it best requires some thinking.

My point being, we cannot afford to add several minutes of linking time for each 'need whole blender' tests, so why not just put those directly as part of Blender itself?

Trying to make Blender more “mockable” might be a nice goal ultimately but I see this as a much more involved project, and we'll still end up with cases where we'll need to link most of blender parts anyway…

@dr.sybren I think you could add C-required unittests as part of Blender build itself, expose the main test function as a python call, and use python script to do the work? C-part of tests being only actualy built when gtests are enabled, of course. Note that this is only a very rough random idea, we could do that in many different ways/code layout, how to do it best requires some thinking. My point being, we cannot afford to add several minutes of linking time for each 'need whole blender' tests, so why not just put those directly as part of Blender itself? Trying to make Blender more “mockable” might be a nice goal ultimately but I see this as a much more involved project, and we'll still end up with cases where we'll need to link most of blender parts anyway…

I agree this is a problem we need to solve.

Using Python where possible is fine, though if it requires adding RNA/Python API functions I'd rather just have the test written in C. Writing and maintaining tests should be as simple as possible.

An alternative would be to only compile one test binary instead of multiple, though including it as part of the Blender executable seems fine if there's a good way to do that.

I agree this is a problem we need to solve. Using Python where possible is fine, though if it requires adding RNA/Python API functions I'd rather just have the test written in C. Writing and maintaining tests should be as simple as possible. An alternative would be to only compile one test binary instead of multiple, though including it as part of the Blender executable seems fine if there's a good way to do that.

I don't see how's GTest at fault here. Any testing framework will have same issues when you ask to link the entire project for standalone binary. Are we specifically optimizing tests which are linking against all libraries?

To me it seems there two separate issues here:

  1. Compilation of bmesh_test should be as fast as possible, so one can easily iterate over changes in bmesh itself. After D6642 it should be possible to link bmesh_test against bf_bmesh and not pull an entire Blender. If, for some reason, that would still pull too much, i think we have bigger architectural problem.

  2. Iteration over changes in Blender itself should be fast, so you can more easily experiment with things like interface layout or drawing (or anything else where "think ahead - compile once" doesn't really work and you have to experiment a lot). In this case i don't know why you would want to link any other binary. It's not like you'll be running them anyway (and running testsuit could be slower than linking anyway). Not sure this is something to be addressed from our build system side, or just tell people use make blender instead of make.

I don't see how's GTest at fault here. Any testing framework will have same issues when you ask to link the entire project for standalone binary. Are we specifically optimizing tests which are linking against all libraries? To me it seems there two separate issues here: 1. Compilation of `bmesh_test` should be as fast as possible, so one can easily iterate over changes in bmesh itself. After [D6642](https://archive.blender.org/developer/D6642) it should be possible to link `bmesh_test` against `bf_bmesh` and not pull an entire Blender. If, for some reason, that would still pull too much, i think we have bigger architectural problem. 2. Iteration over changes in Blender itself should be fast, so you can more easily experiment with things like interface layout or drawing (or anything else where "think ahead - compile once" doesn't really work and you have to experiment a lot). In this case i don't know why you would want to link any other binary. It's not like you'll be running them anyway (and running testsuit could be slower than linking anyway). Not sure this is something to be addressed from our build system side, or just tell people use `make blender` instead of `make`.

In #73268#854740, @mont29 wrote:
@dr.sybren I think you could add C-required unittests as part of Blender build itself, expose the main test function as a python call, and use python script to do the work? C-part of tests being only actualy built when gtests are enabled, of course.

That doesn't sound unreasonable at all.

Trying to make Blender more “mockable” might be a nice goal ultimately but I see this as a much more involved project, and we'll still end up with cases where we'll need to link most of blender parts anyway…

Yes, this is more of a mindset for future designs than something we can just quickly do.

> In #73268#854740, @mont29 wrote: > @dr.sybren I think you could add C-required unittests as part of Blender build itself, expose the main test function as a python call, and use python script to do the work? C-part of tests being only actualy built when gtests are enabled, of course. That doesn't sound unreasonable at all. > Trying to make Blender more “mockable” might be a nice goal ultimately but I see this as a much more involved project, and we'll still end up with cases where we'll need to link most of blender parts anyway… Yes, this is more of a mindset for future designs than something we can just quickly do.

The #test project was a playground to test subprojects and milestones, it is not a real project :)

The #test project was a playground to test subprojects and milestones, it is not a real project :)
Author
Owner

@Sergey I never said GTests was at fault, indeed any testing framework would give the same 'issue'. Fault here is making tests that require linking most of Blender, we simply cannot afford to have to link potentially tens of binaries like that in the future, with the three current 'all-blender-linked' tests it’s already a serious pain for me to build. Sure make blender and such ease the pain a bit, but those are lacking install, and in general we should not have to use such band-aids.

@brecht indeed a single test binary could be a good fallback, if we cannot get a satisfying 'all in blender' solution. Will try to investigate this a bit more on practical side of things in near future, and come back with some actual proposal…

@Sergey I never said GTests was at fault, indeed any testing framework would give the same 'issue'. Fault here is making tests that require linking most of Blender, we simply cannot afford to have to link potentially tens of binaries like that in the future, with the three current 'all-blender-linked' tests it’s already a serious pain for me to build. Sure `make blender` and such ease the pain a bit, but those are lacking install, and in general we should not have to use such band-aids. @brecht indeed a single test binary could be a good fallback, if we cannot get a satisfying 'all in blender' solution. Will try to investigate this a bit more on practical side of things in near future, and come back with some actual proposal…

I never said GTests was at fault

That's how first sentence of the task reads though :\

Sure make blender and such ease the pain a bit, but those are lacking install, and in general we should not have to use such band-aids.

I don't see make blender being a band-aid here. Why to do any extra work on linking something you don't currently need anyway? Can't become faster than that.

indeed a single test binary could be a good fallback

I don't agree here. Tests themselves should be easy/fast to iterate over. I wouldn't want to wait alembic symbols to be resolved when working on blenloader, for example.

> I never said GTests was at fault That's how first sentence of the task reads though :\ > Sure make blender and such ease the pain a bit, but those are lacking install, and in general we should not have to use such band-aids. I don't see `make blender` being a band-aid here. Why to do any extra work on linking something you don't currently need anyway? Can't become faster than that. > indeed a single test binary could be a good fallback I don't agree here. Tests themselves should be easy/fast to iterate over. I wouldn't want to wait alembic symbols to be resolved when working on blenloader, for example.

In #73268#854888, @Sergey wrote:
2. Iteration over changes in Blender itself should be fast, so you can more easily experiment with things like interface layout or drawing (or anything else where "think ahead - compile once" doesn't really work and you have to experiment a lot). In this case i don't know why you would want to link any other binary. It's not like you'll be running them anyway (and running testsuit could be slower than linking anyway). Not sure this is something to be addressed from our build system side, or just tell people use make blender instead of make.

When I'm working on a core part of Blender, for me iteration means compiling and running all tests every 10 minutes. Same when I'm committing changes or reviewing patches. So for my workflow at least, this task is about making iteration that as fast as possible.

> In #73268#854888, @Sergey wrote: > 2. Iteration over changes in Blender itself should be fast, so you can more easily experiment with things like interface layout or drawing (or anything else where "think ahead - compile once" doesn't really work and you have to experiment a lot). In this case i don't know why you would want to link any other binary. It's not like you'll be running them anyway (and running testsuit could be slower than linking anyway). Not sure this is something to be addressed from our build system side, or just tell people use `make blender` instead of `make`. When I'm working on a core part of Blender, for me iteration means compiling and running all tests every 10 minutes. Same when I'm committing changes or reviewing patches. So for my workflow at least, this task is about making iteration that as fast as possible.

Removed subscriber: @Sergey

Removed subscriber: @Sergey

This is different workflow from what i used and what i would like to be used more.
But since i only do janitory work of fixing wrong usage of cmake you guys would come to agreement faster without me.

This is different workflow from what i used and what i would like to be used more. But since i only do janitory work of fixing wrong usage of cmake you guys would come to agreement faster without me.

This problem is getting worse now with more testing being added, armature and f-curve tests specifically. I would really like to see a solution here. Regardless of what the right workflow may be, each new test adding more than a GB to the build folder with debug builds does not scale.

So I really think we should build the tests that link Blender libraries as part of the Blender binary. @mont29 did you make any progress on this?

This problem is getting worse now with more testing being added, armature and f-curve tests specifically. I would really like to see a solution here. Regardless of what the right workflow may be, each new test adding more than a GB to the build folder with debug builds does not scale. So I really think we should build the tests that link Blender libraries as part of the Blender binary. @mont29 did you make any progress on this?
Member

Added subscriber: @JacquesLucke

Added subscriber: @JacquesLucke
Member

Added subscriber: @Jeroen-Bakker

Added subscriber: @Jeroen-Bakker
Member

Added subscriber: @EAW

Added subscriber: @EAW

Here are some design considerations to discuss & agree upon before we move to the technical side of things. I'll consider tests written in C and C++ as equal for this task (won't keep repeating C/C++).

Scope of This Task

Currently there is a separation between the tests in C (separate executable, their own CLI arguments) and Python. Each test is run by ctest, which handles parallelisation, test selection etc. Do we want to be able to run everything from Blender (i.e. replace ctest as front-end as well)? Personally I would keep this task simple & only address the concrete issues (too many big binaries, slow build).

Running & Selecting Tests

Once the tests are integrated with Blender itself, we should be able to select which tests to run. For this I see a few options:

  • Add CLI options to select & run tests. The way I envision this is that all remaining options are then interpreted by the GTest test runner, and not by Blender.
  • Expose tests + a way to run them to Python. The --python-expr CLI argument can then be used if selection from the CLI is required, or we could expose them in such a way that they are considered subclasses of unittest.UnitTest and discoverable by the Python unit test runner.

My preference would be the former, as it makes it easier to discover the correct CLI arguments (we could extend the --help output when WITH_GTESTS=ON). Integrating with CMake/CTest will also be a bit easier, as it reduces one level of quotes, and I don't see a reason to move away from CTests to Python's unittest runner.

We should also decide on how often we want to restart Blender during the tests. We could run it for every test case, but that would likely be slow & inconvenient. I think it's best to have one invocation for each test suite, in the same way that we now have an invocation for each test executable.

Location of Tests

Currently the tests are placed outside the source directory. We could technically turn this into its own module, but that's probably going to be quite big, and require re-linking of all the tests (including potentially large tests) when a small thing changes. We could make every directory in tests/gtests their own module, so they can be built & linked separately. Alternatively, we could move the tests into their respective modules in source, so for example tests/gtests/blenkernel would then move to source/blender/blenkernel/tests.

Personally I'm in favour of the latter. It's akin to what Golang is doing: tests are sitting next to the file they're testing, and thefile.go is tested by thefile_tests.go. This has special compiler support though, and I wouldn't want to go that far with Blender. However, I found it to be rather comfortable to have the actual code and the test for that code sitting in relatively close proximity.

Here are some design considerations to discuss & agree upon before we move to the technical side of things. I'll consider tests written in C and C++ as equal for this task (won't keep repeating C/C++). ### Scope of This Task Currently there is a separation between the tests in C (separate executable, their own CLI arguments) and Python. Each test is run by `ctest`, which handles parallelisation, test selection etc. Do we want to be able to run everything from Blender (i.e. replace ctest as front-end as well)? Personally I would keep this task simple & only address the concrete issues (too many big binaries, slow build). ### Running & Selecting Tests Once the tests are integrated with Blender itself, we should be able to select which tests to run. For this I see a few options: - Add CLI options to select & run tests. The way I envision this is that all remaining options are then interpreted by the GTest test runner, and not by Blender. - Expose tests + a way to run them to Python. The `--python-expr` CLI argument can then be used if selection from the CLI is required, or we could expose them in such a way that they are considered subclasses of `unittest.UnitTest` and discoverable by the Python unit test runner. My preference would be the former, as it makes it easier to discover the correct CLI arguments (we could extend the `--help` output when `WITH_GTESTS=ON`). Integrating with CMake/CTest will also be a bit easier, as it reduces one level of quotes, and I don't see a reason to move away from CTests to Python's unittest runner. We should also decide on how often we want to restart Blender during the tests. We could run it for every test case, but that would likely be slow & inconvenient. I think it's best to have one invocation for each test suite, in the same way that we now have an invocation for each test executable. ### Location of Tests Currently the tests are placed outside the `source` directory. We could technically turn this into its own module, but that's probably going to be quite big, and require re-linking of all the tests (including potentially large tests) when a small thing changes. We could make every directory in `tests/gtests` their own module, so they can be built & linked separately. Alternatively, we could move the tests into their respective modules in `source`, so for example `tests/gtests/blenkernel` would then move to `source/blender/blenkernel/tests`. Personally I'm in favour of the latter. It's akin to what Golang is doing: tests are sitting next to the file they're testing, and `thefile.go` is tested by `thefile_tests.go`. This has special compiler support though, and I wouldn't want to go that far with Blender. However, I found it to be rather comfortable to have the actual code and the test for that code sitting in relatively close proximity.

Agreed with @dr.sybren on all points.

GTest supports parsing arguments --gtest_list_tests, --gtest_filter, etc:
https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#running-test-programs-advanced-options

So perhaps we could just rely on that.

Agreed with @dr.sybren on all points. GTest supports parsing arguments `--gtest_list_tests`, `--gtest_filter`, etc: https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#running-test-programs-advanced-options So perhaps we could just rely on that.
Member

Added subscriber: @LazyDodo

Added subscriber: @LazyDodo
Member

Funny question? how did we jump to "lets put the test inside blender!" ? I feel there may have been some internal discussion I may have missed?

Why was the idea of a single test runner binary rejected? ie I see no reason we cannot stick a bunch of tests into a single hosting binary (one that is not blender...) I don't mind a long link time once (or twice if you want to count the linking of blender itself),however 8x in parallel each munching up 7-8 gigs of ram each is a different story, twice however seems perfectly fine...

kinda feels like, putting it all in the blender binary is taking it a bit too far? and way more hassle than needed?

Funny question? how did we jump to "lets put the test inside blender!" ? I feel there may have been some internal discussion I may have missed? Why was the idea of a single test runner binary rejected? ie I see no reason we cannot stick a bunch of tests into a single hosting binary (one that is not blender...) I don't mind a long link time once (or twice if you want to count the linking of blender itself),however 8x in parallel each munching up 7-8 gigs of ram each is a different story, twice however seems perfectly fine... kinda feels like, putting it all in the blender binary is taking it a bit *too* far? and way more hassle than needed?
Author
Owner

Also agree with @dr.sybren essentially.

@LazyDodo I do not see why that would be too far? Extra overhead (size) in final binary should be very small compared to Blender size these days, and even two linking tasks eating gigs of ram are already an issue on e.g. almost any laptop sold currently (since getting anything beyond 16GB remains a challenge)… And twice as slow is not fine, when you have to build and test repetitively. ;)

Also agree with @dr.sybren essentially. @LazyDodo I do not see why that would be too far? Extra overhead (size) in final binary should be very small compared to Blender size these days, and even two linking tasks eating gigs of ram are already an issue on e.g. almost any laptop sold currently (since getting anything beyond 16GB remains a challenge)… And twice as slow is not fine, when you have to build and test repetitively. ;)

I was thinking of including the tests in the same compilation unit as the main code (so libbf_blenkernel.a for example). However, for building the tests we suppress a lot of warnings via the remove_strict_flags() CMake macro. Without this, we get lots of warnings like these:

In file included from /home/sybren/workspace/blender-git/blender/tests/gtests/testing/testing.h:8:0,
                 from /home/sybren/workspace/blender-git/blender/source/blender/blenkernel/tests/BKE_fcurve_test.cc:18:
/home/sybren/workspace/blender-git/blender/extern/gtest/include/gtest/gtest.h:1955:6: warning: "GTEST_DONT_DEFINE_ASSERT_EQ" is not defined, evaluates to 0 [-Wundef]
 #if !GTEST_DONT_DEFINE_ASSERT_EQ
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/sybren/workspace/blender-git/blender/extern/gtest/include/gtest/gtest.h:1959:6: warning: "GTEST_DONT_DEFINE_ASSERT_NE" is not defined, evaluates to 0 [-Wundef]
 #if !GTEST_DONT_DEFINE_ASSERT_NE
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/sybren/workspace/blender-git/blender/extern/gtest/include/gtest/gtest.h:1963:6: warning: "GTEST_DONT_DEFINE_ASSERT_LE" is not defined, evaluates to 0 [-Wundef]
 #if !GTEST_DONT_DEFINE_ASSERT_LE
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~

These are not warnings I would like to suppress for the main Blender source, though, so I would suggest putting the tests in their own compilation unit.

I was thinking of including the tests in the same compilation unit as the main code (so `libbf_blenkernel.a` for example). However, for building the tests we suppress a lot of warnings via the `remove_strict_flags()` CMake macro. Without this, we get lots of warnings like these: ``` In file included from /home/sybren/workspace/blender-git/blender/tests/gtests/testing/testing.h:8:0, from /home/sybren/workspace/blender-git/blender/source/blender/blenkernel/tests/BKE_fcurve_test.cc:18: /home/sybren/workspace/blender-git/blender/extern/gtest/include/gtest/gtest.h:1955:6: warning: "GTEST_DONT_DEFINE_ASSERT_EQ" is not defined, evaluates to 0 [-Wundef] #if !GTEST_DONT_DEFINE_ASSERT_EQ ^~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/sybren/workspace/blender-git/blender/extern/gtest/include/gtest/gtest.h:1959:6: warning: "GTEST_DONT_DEFINE_ASSERT_NE" is not defined, evaluates to 0 [-Wundef] #if !GTEST_DONT_DEFINE_ASSERT_NE ^~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/sybren/workspace/blender-git/blender/extern/gtest/include/gtest/gtest.h:1963:6: warning: "GTEST_DONT_DEFINE_ASSERT_LE" is not defined, evaluates to 0 [-Wundef] #if !GTEST_DONT_DEFINE_ASSERT_LE ^~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` These are not warnings I would like to suppress for the main Blender source, though, so I would suggest putting the tests in their own compilation unit.

Why is it necessary to put the tests along-side the regular source files?
(in the source code).

This has the down-side that tests will show up when searching the code-base (something that's very common and done often the time during development),

can't changes to linking to done without moving the tests into source/ ?

Why is it necessary to put the tests along-side the regular source files? (in the source code). This has the down-side that tests will show up when searching the code-base (something that's very common and done often the time during development), can't changes to linking to done without moving the tests into `source/` ?

In #73268#926062, @ideasman42 wrote:
Why is it necessary to put the tests along-side the regular source files?

It is not necessary, as anything can be worked around and modified. In my experience it's nice to have tests & regular code setting next to each other. It makes the tests more visible and invites people to write more tests.

From a technical point of view, it helps with the CMake configuration. By having the tests in a subdirectory of the regular source, it inherits all the options for valid include paths, etc. This reduces the need to sync settings between the two.

> In #73268#926062, @ideasman42 wrote: > Why is it necessary to put the tests along-side the regular source files? It is not necessary, as anything can be worked around and modified. In my experience it's nice to have tests & regular code setting next to each other. It makes the tests more visible and invites people to write more tests. From a technical point of view, it helps with the CMake configuration. By having the tests in a subdirectory of the regular source, it inherits all the options for valid include paths, etc. This reduces the need to sync settings between the two.

Was it considered to build a blender library (with all symbols made public) and link tests against this?

While it's some duplication, it's only done once, instead of including much of blender in each binary.

Re-linking Blender is fairly slow these days, making an edit-build-test cycle slow if tests are running from Blender.

Was it considered to build a blender library (with all symbols made public) and link tests against this? While it's some duplication, it's only done once, instead of including much of blender in each binary. Re-linking Blender is fairly slow these days, making an edit-build-test cycle slow if tests are running from Blender.

That wouldn't matter much if we use static linking, then each test executable would still be huge.

That wouldn't matter much if we use static linking, then each test executable would still be huge.

I assume Campbell is talking about dynamic linking? But we have problems with symbol conflicts and I wanted to hide more symbols (see #76442).

I would put all tests that need Blender libraries in a single statically linked binary separate from the Blender binary to solve the most urgent problem. Shipping tests in release builds is not something we should do I think, I hadn't considered that earlier.

Moving to shared libraries I think is fine to improve Blender link time as a whole, especially if we can split things up into smaller libraries. But I expect that will open a can of worms and will be a bigger project?

I assume Campbell is talking about dynamic linking? But we have problems with symbol conflicts and I wanted to hide more symbols (see #76442). I would put all tests that need Blender libraries in a single statically linked binary separate from the Blender binary to solve the most urgent problem. Shipping tests in release builds is not something we should do I think, I hadn't considered that earlier. Moving to shared libraries I think is fine to improve Blender link time as a whole, especially if we can split things up into smaller libraries. But I expect that will open a can of worms and will be a bigger project?
Member

Looked into that previously, to make that work it'll be a much much bigger project

  • Circular dependencies between the libs are going to be problematic (ie i can't link A to B if B doesn't exist yet, but i can't build B without having A) and we have quite a few of those
  • Exports, MSVC doesn't export anything out of a shared lib, we'll have to either attribute every function we'd like to export or resort to the cmake WINDOWS_EXPORT_ALL_SYMBOLS option, which i have to admit i have never tried on a project the size of blender.
  • Our dependencies between libs are... yeah... here's the deps in just the lite configuration from about 2 years ago. (based on parsing out the imports/exports from the object files, i do not trust us to have properly modeled the library inter-dependencies in cmake)

{F8518716, width=75%}

Looked into that previously, to make that work it'll be a much much bigger project - Circular dependencies between the libs are going to be problematic (ie i can't link A to B if B doesn't exist yet, but i can't build B without having A) and we have quite a few of those - Exports, MSVC doesn't export anything out of a shared lib, we'll have to either attribute every function we'd like to export or resort to the cmake `WINDOWS_EXPORT_ALL_SYMBOLS` option, which i have to admit i have never tried on a project the size of blender. - Our dependencies between libs are... yeah... here's the deps in just the lite configuration from about 2 years ago. (based on parsing out the imports/exports from the object files, i do not trust us to have properly modeled the library inter-dependencies in cmake) {[F8518716](https://archive.blender.org/developer/F8518716/image.png), width=75%}
Member

~~I think some reductions in link time can be had by moving some of the deps to shared libs, USD and LLVM would be on top of my list there, we've been statically linking since before i started, so i can't say i have much of a historic perspective on why we're doing that? ~~ Disabling USD+OSL+LLVM made no difference in link time so moving them to dynamic is unlikely to offer any relief in the area of link times.

~~I think some reductions in link time can be had by moving some of the deps to shared libs, USD and LLVM would be on top of my list there, we've been statically linking since before i started, so i can't say i have much of a historic perspective on why we're doing that? ~~ Disabling USD+OSL+LLVM made no difference in link time so moving them to dynamic is unlikely to offer any relief in the area of link times.

Added subscriber: @damd

Added subscriber: @damd

Added subscriber: @Sergey

Added subscriber: @Sergey

I had a discussion about this with @Sergey yesterday, and we came to the conclusion that (for us anyway) the best place to put the tests would be right next to the files that are tested. This means you'd get something like:

  • source/blender/depsgraph/intern/builder/deg_builder_remove_noop.h
  • source/blender/depsgraph/intern/builder/deg_builder_remove_noop.cc
  • source/blender/depsgraph/intern/builder/deg_builder_remove_noop_test.cc

My earlier proposal to place tests in a directory next to the intern directory (so in the above example source/blender/depsgraph/tests) isn't so nice when there are subdirectories inside intern.

Another advantage of having tests in the same module as the files under test, is that they can directly use a non-public interface. Without this, we'd have to allow the test module to include from internal directories of another module.

I had a discussion about this with @Sergey yesterday, and we came to the conclusion that (for us anyway) the best place to put the tests would be right next to the files that are tested. This means you'd get something like: - source/blender/depsgraph/intern/builder/deg_builder_remove_noop.h - source/blender/depsgraph/intern/builder/deg_builder_remove_noop.cc - source/blender/depsgraph/intern/builder/deg_builder_remove_noop_test.cc My earlier proposal to place tests in a directory next to the `intern` directory (so in the above example `source/blender/depsgraph/tests`) isn't so nice when there are subdirectories inside `intern`. Another advantage of having tests in the same module as the files under test, is that they can directly use a non-public interface. Without this, we'd have to allow the test module to include from internal directories of another module.
Author
Owner

I don’t really mind where the test files are placed, as long as they are close enough for actual code to be "obviously" visible for anyone working there. So am also fine with having them in the form of foo_bar_test.cc next to foo_bar.cc implementation.

We may also want to allow both actually? Some intern/ directories are already nicely organized in sub-directories, while others (like BKE or RNA or modifiers) tend to turn into a giant flat storage of files, not sure keeping tests files on same level is best idea then?

I don’t really mind where the test files are placed, as long as they are close enough for actual code to be "obviously" visible for anyone working there. So am also fine with having them in the form of `foo_bar_test.cc` next to `foo_bar.cc` implementation. We *may* also want to allow both actually? Some `intern/` directories are already nicely organized in sub-directories, while others (like BKE or RNA or modifiers) tend to turn into a giant flat storage of files, not sure keeping tests files on same level is best idea then?

We may also want to allow both actually? Some intern/ directories are already nicely organized in sub-directories, while others (like BKE or RNA or modifiers) tend to turn into a giant flat storage of files, not sure keeping tests files on same level is best idea then?

There are some areas where having tests/ next to its intern/ might work best (compositor? bmesh?), but generally we should split those giant flat storage of files. For example, tracking, making, subdivision, multires are all to be moved out of BKE.

Also, it is actually easier to find foo_test.cc for currently opened foo.cc in smart editor even in the case of giant flat storage of files: foo_test.cc would be next to the current file in the explorer and you can just click on it. If the test is in separate directory it will be more scrolling around (sure you can use fuzzy search, but that's usually slower than simple click on something you already see on the screen).

There are some areas where "module-global" (test/ next to intern/) would work better (compositor? bmesh?) but would advice having those more of an exception. Simply from "predictability" point of view: so it's more commonly clear where to find test corresponding test.

> We *may* also want to allow both actually? Some `intern/` directories are already nicely organized in sub-directories, while others (like BKE or RNA or modifiers) tend to turn into a giant flat storage of files, not sure keeping tests files on same level is best idea then? There are some areas where having `tests/` next to its `intern/` might work best (compositor? bmesh?), but generally we should split those giant flat storage of files. For example, tracking, making, subdivision, multires are all to be moved out of BKE. Also, it is actually easier to find `foo_test.cc` for currently opened `foo.cc` in smart editor even in the case of giant flat storage of files: `foo_test.cc` would be next to the current file in the explorer and you can just click on it. If the test is in separate directory it will be more scrolling around (sure you can use fuzzy search, but that's usually slower than simple click on something you already see on the screen). There are some areas where "module-global" (`test/` next to `intern/`) would work better (compositor? bmesh?) but would advice having those more of an exception. Simply from "predictability" point of view: so it's more commonly clear where to find test corresponding test.
Member

There is currently a separate directory, tests/gtests/blenlib with a lot of blenlib tests, each of which need only link against blenlib and very little else. Would the intention be to move those files to beside their source files too? And continue to have a way to build them without including all of blender?

There is currently a separate directory, tests/gtests/blenlib with a lot of blenlib tests, each of which need only link against blenlib and very little else. Would the intention be to move those files to beside their source files too? And continue to have a way to build them without including all of blender?

In #73268#926265, @brecht wrote:
I would put all tests that need Blender libraries in a single statically linked binary separate from the Blender binary to solve the most urgent problem.

I agree. That's already a step forward from the current situation, although it doesn't make @mont29 happy. It also opens the door to adding more tests without significant impact on the build process.

In #73268#934899, @howardt wrote:
There is currently a separate directory, tests/gtests/blenlib with a lot of blenlib tests, each of which need only link against blenlib and very little else. Would the intention be to move those files to beside their source files too?

I would be in favour of that, yes. IMO it's not worth the hassle having some test files next to the files under test, and other test files in a separate directory. If we keep them in tests/gtests/blenlib there will always be the confusion of where to place new tests.

And continue to have a way to build them without including all of blender?

I appreciate the speed of testing that this gives. However, I wouldn't want to have two ways to build & run tests (i.e. as separate binary like we have now, and with the single-big-test-runner). This would mean that some tests would be in their own binary, whereas others would be part of the big test runner. That's fine by me, as long as it's clear which test ends up where, and one ctest invocation can run all of them

> In #73268#926265, @brecht wrote: > I would put all tests that need Blender libraries in a single statically linked binary separate from the Blender binary to solve the most urgent problem. I agree. That's already a step forward from the current situation, although it doesn't make @mont29 happy. It also opens the door to adding more tests without significant impact on the build process. > In #73268#934899, @howardt wrote: > There is currently a separate directory, tests/gtests/blenlib with a lot of blenlib tests, each of which need only link against blenlib and very little else. Would the intention be to move those files to beside their source files too? I would be in favour of that, yes. IMO it's not worth the hassle having some test files next to the files under test, and other test files in a separate directory. If we keep them in `tests/gtests/blenlib` there will always be the confusion of where to place new tests. > And continue to have a way to build them without including all of blender? I appreciate the speed of testing that this gives. However, I wouldn't want to have two ways to build & run tests (i.e. as separate binary like we have now, and with the single-big-test-runner). This would mean that some tests would be in their own binary, whereas others would be part of the big test runner. That's fine by me, as long as it's clear which test ends up where, and one `ctest` invocation can run all of them
Author
Owner

@dr.sybren if you think putting all tests in a single binary alongside Blender one is much easier/quicker to do than putting everything in Blender itself, then by all mean, please do. Everything is good to take, and linking two monsters will already be an order of magnitude better than linking eight or more!

@dr.sybren if you think putting all tests in a single binary alongside Blender one is much easier/quicker to do than putting everything in Blender itself, then by all mean, please do. Everything is good to take, and linking two monsters will already be an order of magnitude better than linking eight or more!

I have updated D7649 with what was discussed above.

I have updated [D7649](https://archive.blender.org/developer/D7649) with what was discussed above.
Sybren A. Stüvel self-assigned this 2020-06-16 14:28:03 +02:00

This issue was referenced by 53d203dea8

This issue was referenced by 53d203dea8230da4e80f3cc61468a4e24ff6759c

Changed status from 'Confirmed' to: 'Resolved'

Changed status from 'Confirmed' to: 'Resolved'

I'm closing this task, as remaining tests have now also been ported.

I'm closing this task, as remaining tests have now also been ported.
Sign in to join this conversation.
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Code Documentation
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Viewport & EEVEE
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Asset Browser Project
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Module
Viewport & EEVEE
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Severity
High
Severity
Low
Severity
Normal
Severity
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
13 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#73268
No description provided.