[PATCH] D20578: [esan|wset] Iterate all memory to compute the total working set

[Mike Aizatsky via llvm-commits]

aizatsky added inline comments.

================
Comment at: lib/esan/working_set.cpp:79
@@ -77,1 +78,3 @@
 
+static u32 processShadowRegion(u32 BitIdx, uptr ShadowStart, uptr ShadowEnd) {
+  u32 WorkingSetSize = 0;
----------------
This function changes shadow value. Should be at least reflected in the name.

Plus "processShadowRegion" is not very telling.

================
Comment at: lib/esan/working_set.cpp:105
@@ +104,3 @@
+// We also clear the lowest bits (most recent working set snapshot).
+static u32 processShadowMemory(u32 BitIdx) {
+  u32 WorkingSetSize = 0;
----------------
Same.

E.g. "computeWorkingSizeAndClearRecent".

================
Comment at: lib/esan/working_set.cpp:130
@@ +129,3 @@
+  // We need a constant to avoid software divide support:
+  static const u32 line_size = 64;
+  CHECK(line_size == getFlags()->cache_line_size);
----------------
symbolic constant for 64 please.

================
Comment at: lib/esan/working_set.cpp:132
@@ +131,3 @@
+  CHECK(line_size == getFlags()->cache_line_size);
+  const u32 KilobyteCachelines = (0x1 << 10) / line_size;
+  static const u32 MegabyteCachelines = KilobyteCachelines << 10;
----------------
static

================
Comment at: lib/esan/working_set_posix.cpp:69
@@ -68,5 +68,3 @@
   if (SigNum == SIGSEGV) {
-
-    // TODO: Add shadow memory fault detection and handling.
-
-    if (AppSigAct.sigaction) {
+    // We rely on si_addr being filled in (thus we do not support old kernels).
+    siginfo_t *SigInfo = (siginfo_t *)Info;
----------------
Should this be working_set_linux.cpp then?


http://reviews.llvm.org/D20578