[PATCH] D34277: [analyzer] Bump default performance thresholds?
This revision was automatically updated to reflect the committed changes.
Closed by commit rL305900: [analyzer] Bump a few default performance
thresholds. (authored by dergachev).
Changed prior to commit:
https://reviews.llvm.org/D34277?vs=102816=103349#toc
Repository:
rL LLVM
https://reviews.llvm.org/D34277
Files:
cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
cfe/trunk/test/Analysis/analyzer-config.c
cfe/trunk/test/Analysis/analyzer-config.cpp
Index: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -293,7 +293,7 @@
DefaultValue = 4;
break;
case UMK_Deep:
-DefaultValue = 50;
+DefaultValue = 100;
break;
}
@@ -332,7 +332,7 @@
DefaultValue = 75000;
break;
case UMK_Deep:
-DefaultValue = 15;
+DefaultValue = 225000;
break;
}
MaxNodesPerTopLevelFunction = getOptionAsInteger("max-nodes",
DefaultValue);
Index: cfe/trunk/test/Analysis/analyzer-config.cpp
===
--- cfe/trunk/test/Analysis/analyzer-config.cpp
+++ cfe/trunk/test/Analysis/analyzer-config.cpp
@@ -30,8 +30,8 @@
// CHECK-NEXT: ipa = dynamic-bifurcate
// CHECK-NEXT: ipa-always-inline-size = 3
// CHECK-NEXT: leak-diagnostics-reference-allocation = false
-// CHECK-NEXT: max-inlinable-size = 50
-// CHECK-NEXT: max-nodes = 15
+// CHECK-NEXT: max-inlinable-size = 100
+// CHECK-NEXT: max-nodes = 225000
// CHECK-NEXT: max-times-inline-large = 32
// CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14
// CHECK-NEXT: mode = deep
Index: cfe/trunk/test/Analysis/analyzer-config.c
===
--- cfe/trunk/test/Analysis/analyzer-config.c
+++ cfe/trunk/test/Analysis/analyzer-config.c
@@ -19,8 +19,8 @@
// CHECK-NEXT: ipa = dynamic-bifurcate
// CHECK-NEXT: ipa-always-inline-size = 3
// CHECK-NEXT: leak-diagnostics-reference-allocation = false
-// CHECK-NEXT: max-inlinable-size = 50
-// CHECK-NEXT: max-nodes = 15
+// CHECK-NEXT: max-inlinable-size = 100
+// CHECK-NEXT: max-nodes = 225000
// CHECK-NEXT: max-times-inline-large = 32
// CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14
// CHECK-NEXT: mode = deep
Index: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -293,7 +293,7 @@
DefaultValue = 4;
break;
case UMK_Deep:
-DefaultValue = 50;
+DefaultValue = 100;
break;
}
@@ -332,7 +332,7 @@
DefaultValue = 75000;
break;
case UMK_Deep:
-DefaultValue = 15;
+DefaultValue = 225000;
break;
}
MaxNodesPerTopLevelFunction = getOptionAsInteger("max-nodes", DefaultValue);
Index: cfe/trunk/test/Analysis/analyzer-config.cpp
===
--- cfe/trunk/test/Analysis/analyzer-config.cpp
+++ cfe/trunk/test/Analysis/analyzer-config.cpp
@@ -30,8 +30,8 @@
// CHECK-NEXT: ipa = dynamic-bifurcate
// CHECK-NEXT: ipa-always-inline-size = 3
// CHECK-NEXT: leak-diagnostics-reference-allocation = false
-// CHECK-NEXT: max-inlinable-size = 50
-// CHECK-NEXT: max-nodes = 15
+// CHECK-NEXT: max-inlinable-size = 100
+// CHECK-NEXT: max-nodes = 225000
// CHECK-NEXT: max-times-inline-large = 32
// CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14
// CHECK-NEXT: mode = deep
Index: cfe/trunk/test/Analysis/analyzer-config.c
===
--- cfe/trunk/test/Analysis/analyzer-config.c
+++ cfe/trunk/test/Analysis/analyzer-config.c
@@ -19,8 +19,8 @@
// CHECK-NEXT: ipa = dynamic-bifurcate
// CHECK-NEXT: ipa-always-inline-size = 3
// CHECK-NEXT: leak-diagnostics-reference-allocation = false
-// CHECK-NEXT: max-inlinable-size = 50
-// CHECK-NEXT: max-nodes = 15
+// CHECK-NEXT: max-inlinable-size = 100
+// CHECK-NEXT: max-nodes = 225000
// CHECK-NEXT: max-times-inline-large = 32
// CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14
// CHECK-NEXT: mode = deep
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34277: [analyzer] Bump default performance thresholds?
zaks.anna added a comment. > In particular, there are patches to generate more symbolic expressions, that > could also degrade the performance (but fix some fixmes along the way). The patch you are talking about might be promising, but needs much more investigation and tuning for performance vs issues found. I do not think we should block this patch until the investigation is done. https://reviews.llvm.org/D34277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34277: [analyzer] Bump default performance thresholds?
NoQ added a comment. Gabor makes such a good point. Maybe we should commit the zombie symbols patch as well (: https://reviews.llvm.org/D34277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34277: [analyzer] Bump default performance thresholds?
a.sidorin accepted this revision. a.sidorin added a comment. Ok, I hope this will work. Anyway, we can always revert this patch in case of any problems. https://reviews.llvm.org/D34277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34277: [analyzer] Bump default performance thresholds?
xazax.hun added a comment. While I have no objections, I am wondering whether this is the good way to spend the performance budget. In particular, there are patches to generate more symbolic expressions, that could also degrade the performance (but fix some fixmes along the way). https://reviews.llvm.org/D34277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34277: [analyzer] Bump default performance thresholds?
NoQ added a comment. This was an mixture of internal apple projects (user apps, drivers, deamons, whatever) with a relatively balanced selection of languages and levels of analyzer adoption. They amounted to ~16 hours of analysis CPU time (i.e. 4 hours on a quad-core machine per run). I've also ran it on LLVM separately, and had similar observations. I'm totally welcoming the feedback from other codebases! In https://reviews.llvm.org/D34277#782605, @zaks.anna wrote: > > Maybe we should introduce another UMK_* for deeper analysis instead? > > The difference here is not substantial enough to warrant a new level. The > general motivation for bumping these numbers is that we've set the timeouts > many years ago and the hardware improved quite a bit since then. Yeah, the point was mostly about default settings, for people who don't bother to tweak them, and adding more options essentially defeats the purpose. https://reviews.llvm.org/D34277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34277: [analyzer] Bump default performance thresholds?
zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. Once Artem gives more details about the codebase he tested on, I think it would be fine to commit this. We can revert/address concerns later if @a.sidorin or anyone else raises concerns based on further testing on their codebases. @a.sidorin, would this work for you? https://reviews.llvm.org/D34277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34277: [analyzer] Bump default performance thresholds?
zaks.anna added a comment. > Maybe we should introduce another UMK_* for deeper analysis instead? The difference here is not substantial enough to warrant a new level. The general motivation for bumping these numbers is that we've set the timeouts many years ago and the hardware improved quite a bit since then. https://reviews.llvm.org/D34277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34277: [analyzer] Bump default performance thresholds?
a.sidorin added a comment. Hi Artem, Could you tell what code bases did you use to collect your statistics? I'll try to check the patch on our code bases. I think we should be careful about default settings. Maybe we should introduce another UMK_* for deeper analysis instead? https://reviews.llvm.org/D34277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34277: [analyzer] Bump default performance thresholds?
NoQ created this revision.
Because we now have faster CPUs and more RAM and stuff, should we now skew the
balance to finding more bugs?
We could probably make a few rounds of such changes, observing any delayed
feedback from users who use default settings and aren't watching phabricator,
and rolling back in case we degrade dramatically on specific smaller projects.
As the first step, i've recently tested the following changes to default
`-analyzer-option`s:
- `max-nodes`: 15 -> 225000 (+50%) - the limit on the size of the exploded
graph.
- `max-inlinable-size`: 50 -> 100 (+100%) - the limit on the number of CFG
blocks in inlined functions.
Totally, this gives 10% performance degradation and finds 5% more bugs on a
large-ish codebase. `max-inlinable-size` change skews the analyzer to find more
IPA-based bugs than before (+/-5% added/lost), and also overally slightly
improves the number of bugs found; `max-nodes` increase brings back some of
these positives.
Generally, it would also be good to make the analyzer work in a more obvious
manner in terms of why does or doesn't it cover certain paths, inline certain
functions, etc.- currently this is a mess of unobvious heuristics, and if we
could make it less obvious by lifting some of these heuristics, it may be an
additional benefit of this work as well.
https://reviews.llvm.org/D34277
Files:
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -293,7 +293,7 @@
DefaultValue = 4;
break;
case UMK_Deep:
-DefaultValue = 50;
+DefaultValue = 100;
break;
}
@@ -332,7 +332,7 @@
DefaultValue = 75000;
break;
case UMK_Deep:
-DefaultValue = 15;
+DefaultValue = 225000;
break;
}
MaxNodesPerTopLevelFunction = getOptionAsInteger("max-nodes",
DefaultValue);
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -293,7 +293,7 @@
DefaultValue = 4;
break;
case UMK_Deep:
-DefaultValue = 50;
+DefaultValue = 100;
break;
}
@@ -332,7 +332,7 @@
DefaultValue = 75000;
break;
case UMK_Deep:
-DefaultValue = 15;
+DefaultValue = 225000;
break;
}
MaxNodesPerTopLevelFunction = getOptionAsInteger("max-nodes", DefaultValue);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
