Page MenuHome
Differential D16314

tests: Disable lattice_deform_performance test
ClosedPublic
Actions

Authored by Ray Molenkamp (LazyDodo) on Oct 21 2022, 6:14 PM.

Details

Reviewers
Jeroen Bakker (jbakker)
Commits
rBc5b36aa9404b: tests: Disable lattice_deform_performance test
Summary

This has come up a few times on chat but no-one ever acted on it. The lattice_deform_performance test has no business running by default for the following reasons:

  • There's no asserts, the test actually cannot fail.
  • Nothing is monitoring these numbers, even if it were to slow down one day by 20% no-one would notice.
  • It is one of the longer tests in bf_blenkernel_tests responsible for 2979 ms out of 3559 ms total.
  • We all run it on a daily basis with seemingly no benefit whatsoever.

If one were to work on the lattice code it is without a doubt a good thing to have, so I'm not advocating to remove it completely.

Other parts of our test code like the mesh intersection code has performance tests, however those are disabled by default with an ifdef guard, this change adds a similar guard to the lattice_deform_test

Diff Detail

Repository
rB Blender
Branch
tmp_disable_lattice_deform_test (branched from master)
Build Status
Buildable 24373
Build 24373: arc lint + arc unit

Event Timeline

Ray Molenkamp (LazyDodo) requested review of this revision.Oct 21 2022, 6:14 PM
Ray Molenkamp (LazyDodo) created this revision.
Harbormaster completed remote builds in B24373: Diff 57025.Oct 21 2022, 6:14 PM
Hans Goudey (HooglyBoogly) awarded a token.Oct 21 2022, 6:20 PM
Hans Goudey (HooglyBoogly) added a subscriber: Hans Goudey (HooglyBoogly).

What do you think about using a constexpr bool to avoid code rot? Otherwise this test might be skipped in refactors.

Ray Molenkamp (LazyDodo) added a comment.Oct 21 2022, 7:36 PM

I just followed what we had elsewhere, if we want to keep building the code but not run it something like

P3268 Masterwork From Distant Lands
1diff --git a/source/blender/blenkernel/intern/lattice_deform_test.cc b/source/blender/blenkernel/intern/lattice_deform_test.cc
2index c66feedc878..1c15dd1d6a7 100644
3--- a/source/blender/blenkernel/intern/lattice_deform_test.cc
4+++ b/source/blender/blenkernel/intern/lattice_deform_test.cc
5@@ -16,6 +16,11 @@
6 #define DO_PERF_TESTS 0
7
8 #if DO_PERF_TESTS
9+# define LATTICE_SKIP_PERF_TEST
10+#else
11+# define LATTICE_SKIP_PERF_TEST GTEST_SKIP();
12+#endif
13+
14 namespace blender::bke::tests {
15
16 struct LatticeDeformTestContext {
17@@ -71,6 +76,7 @@ static void test_lattice_deform_free(LatticeDeformTestContext *ctx)
18
19 TEST(lattice_deform_performance, performance_no_dvert_1)
20 {
21+ LATTICE_SKIP_PERF_TEST
22 const int32_t num_items = 1;
23 LatticeDeformTestContext ctx = {dna::shallow_zero_initialize()};
24 RandomNumberGenerator rng;
25@@ -80,6 +86,7 @@ TEST(lattice_deform_performance, performance_no_dvert_1)
26 }
27 TEST(lattice_deform_performance, performance_no_dvert_1000)
28 {
29+ LATTICE_SKIP_PERF_TEST
30 const int32_t num_items = 1000;
31 LatticeDeformTestContext ctx = {dna::shallow_zero_initialize()};
32 RandomNumberGenerator rng;
33@@ -89,6 +96,7 @@ TEST(lattice_deform_performance, performance_no_dvert_1000)
34 }
35 TEST(lattice_deform_performance, performance_no_dvert_10000)
36 {
37+ LATTICE_SKIP_PERF_TEST
38 const int32_t num_items = 10000;
39 LatticeDeformTestContext ctx = {dna::shallow_zero_initialize()};
40 RandomNumberGenerator rng;
41@@ -98,6 +106,7 @@ TEST(lattice_deform_performance, performance_no_dvert_10000)
42 }
43 TEST(lattice_deform_performance, performance_no_dvert_100000)
44 {
45+ LATTICE_SKIP_PERF_TEST
46 const int32_t num_items = 100000;
47 LatticeDeformTestContext ctx = {dna::shallow_zero_initialize()};
48 RandomNumberGenerator rng;
49@@ -107,6 +116,7 @@ TEST(lattice_deform_performance, performance_no_dvert_100000)
50 }
51 TEST(lattice_deform_performance, performance_no_dvert_1000000)
52 {
53+ LATTICE_SKIP_PERF_TEST
54 const int32_t num_items = 1000000;
55 LatticeDeformTestContext ctx = {dna::shallow_zero_initialize()};
56 RandomNumberGenerator rng;
57@@ -116,6 +126,7 @@ TEST(lattice_deform_performance, performance_no_dvert_1000000)
58 }
59 TEST(lattice_deform_performance, performance_no_dvert_10000000)
60 {
61+ LATTICE_SKIP_PERF_TEST
62 const int32_t num_items = 10000000;
63 LatticeDeformTestContext ctx = {dna::shallow_zero_initialize()};
64 RandomNumberGenerator rng;
65@@ -125,4 +136,3 @@ TEST(lattice_deform_performance, performance_no_dvert_10000000)
66 }
67
68 } // namespace blender::bke::tests
69-#endif

could work, the docs say you should be able to control this from a test fixture, but i couldn't get that to work, it just kept on running the tests...

Hans Goudey (HooglyBoogly) added a comment.Oct 21 2022, 8:02 PM

Oh, I see, looking at this properly I think my suggestion is bad. This is a very small amount of code anyway.

Jeroen Bakker (jbakker) accepted this revision.Nov 8 2022, 8:59 AM

Yes, eventually this should be tracked and reported on. Something we want to add eventually and has been discussed here, but other prios...

This revision is now accepted and ready to land.Nov 8 2022, 8:59 AM
Closed by commit rBc5b36aa9404b: tests: Disable lattice_deform_performance test (authored by Ray Molenkamp (LazyDodo)). · Explain WhyNov 11 2022, 9:50 PM
This revision was automatically updated to reflect the committed changes.
Ray Molenkamp (LazyDodo) added a commit: rBc5b36aa9404b: tests: Disable lattice_deform_performance test.

Revision Contents
Changeset List

PathSize
source/
blender/
blenkernel/
intern/
4 lines

Diff 57025

View Options

source/blender/blenkernel/intern/lattice_deform_test.cc

Loading...