Skip to content

Commit d136153

Browse files
committed
Rewrote name lookup, should improve perf of #904
After a couple of passes of optimizing get_declaration_of to make its traversal of the symbol table more efficient, I decided to rewrite it entirely to build the information during the initial visitation and not scour at the symbol table afterwards to rediscover the information. The code generation for all `*.cpp2` and `*.h2` files in the project is unchanged, so it seems likely that this rewrite is not introducing bugs. If we do find some, they should be easier to fix as the lookup logic is now more direct and built and consumed in just two places. To prevent accidental behavior change, I exhaustively checked every call to get_declaration_of to check whether it returned a different result before and after this change. There were quite a few during debugging which pointed out where I'd overlooked a lookup case, and those are now all fixed. There remained only a few (<10) calls in this entire corpus where g_d_o is now giving a different result, but all are cases where g_d_o was not being consistent previously (repeated calls with the same arguments in the same execution sometimes gave a result and sometimes gave null) and it's now giving the same answer consistently that it gave "sometimes" before. So I think this change is fixing those as (latent) bugs... latent because the compilation output before/after is unchanged, because the previous inconsistency was in cases that didn't actually affect compilation behavior.
1 parent 130f149 commit d136153

File tree

3 files changed

+226
-456
lines changed

3 files changed

+226
-456
lines changed

include/cpp2util.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,12 @@ template <typename T>
511511
requires requires { *std::declval<T&>(); }
512512
using deref_t = decltype(*std::declval<T&>());
513513

514+
// Guaranteed to be a total order, unlike built-in operator== for T*
515+
template <typename T>
516+
auto pointer_eq(T const* a, T const* b) {
517+
return std::compare_three_way{}(a, b) == std::strong_ordering::equal;
518+
}
519+
514520

515521
//-----------------------------------------------------------------------
516522
//

source/common.h

Lines changed: 2 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,90 +1248,8 @@ std::vector<stackinstr::entry> stackinstr::largest;
12481248
// timer: a little profiling timer to measure time spent in
12491249
// specific sections of code
12501250
//
1251-
// This is a tool for cppfront developers to measure the run-time of
1252-
// specific parts of cppfront code.
1253-
//
1254-
// Do still use a profiler too. But once you know where to measure,
1255-
// this has some advantages:
1256-
//
1257-
// - Precise: It's easy to accurately measure specific parts of
1258-
// code across the run of the program, not relying on sampling to
1259-
// representatively hit those regions
1260-
//
1261-
// - Flexible: You can have as many timers as you want, each with
1262-
// a clearly readable name, including nested timers (see example
1263-
// below)
1264-
//
1265-
// - Portable: It's part of cppfront itself, no separate tool needed
1266-
//
1267-
// - Integrated: Reports are printed automatically by -verbose
1268-
//
1269-
// Usage guide
1270-
// -----------
1271-
//
1272-
// To measure the total time spent in a specific section of code, create
1273-
// a scope_timer guard object in that scope and give it a readable
1274-
// string name that will remind you what it's measuring.
1275-
//
1276-
// Example: Today I wanted to measure how long get_declaration_of is
1277-
// taking. So add this line at the start of that function's body:
1278-
//
1279-
// auto timer = scope_timer("get_declaration_of");
1280-
//
1281-
// Recompile cppfront, then run with -verbose:
1282-
//
1283-
// cppfront pure2-last-use.cpp2 -p -verb
1284-
//
1285-
// Sample output:
1286-
//
1287-
// pure2-last-use.cpp2... ok (all Cpp2, passes safety checks)
1288-
// Cpp1 0 lines
1289-
// Cpp2 1,050 lines (100%)
1290-
// Time 1,629 ms
1291-
// 1,120 ms in get_declaration_of
1292-
//
1293-
// The first "Time:" is the total run time of the compiland. It is then
1294-
// followed by any active scope_timer names, in alphabetical order
1295-
// (you can choose names so that related timers appear together).
1296-
//
1297-
// Here, this one function is taking most of the total runtime. It
1298-
// consists of two major loops, one run before the other, so we can
1299-
// measure the cost of each part, using names that will be
1300-
// reported together and in understandable order with -verbose:
1301-
//
1302-
// - The function's first loop is to find a starting point, so
1303-
// enclose that in { } with a scope_timer:
1304-
//
1305-
// {
1306-
// auto timer1 = scope_timer("get_declaration_of step 1, initial find loop");
1307-
// /* the code I want to measure */
1308-
// }
1309-
//
1310-
// - Immediately after that, install a second timer to measure the
1311-
// second loop which covers the entire rest of the function body:
1312-
//
1313-
// auto timer2 = scope_timer("get_declaration_of step 2, rest of lookup");
1314-
// /* followed by the rest of the function's body */
1315-
//
1316-
// - And, since it's easy, throw in a third timer to measure one
1317-
// sub-part of the step 2 loop... right before the 'move this'
1318-
// branch we can add this to measure the time spent in oly that
1319-
// trailing 1/3 of each loop iteration (starting and stopping the
1320-
// timer at those points in each loop iteration to measure just
1321-
// the sum of all those loop fragments):
1322-
//
1323-
// auto timer2b = scope_timer("get_declaration_of step 2b, 'move this' branch");
1324-
//
1325-
// Recompile cppfront and run again with -verbose... sample output:
1326-
//
1327-
// pure2-last-use.cpp2... ok (all Cpp2, passes safety checks)
1328-
// Cpp1 0 lines
1329-
// Cpp2 1,050 lines (100%)
1330-
// Time 1,471 ms
1331-
// 1,139 ms in get_declaration_of
1332-
// 540 ms in get_declaration_of step 1, initial find loop
1333-
// 567 ms in get_declaration_of step 2, rest of lookup
1334-
// 148 ms in get_declaration_of step 2b, 'move this' branch
1251+
// Use CPP2_SCOPE_TIMER("unique name") to declare local objects whose
1252+
// total timings will be reported in -verbose in CPP2_DEBUG_BUILD builds.
13351253
//
13361254
//-----------------------------------------------------------------------
13371255
//

0 commit comments

Comments
 (0)