[PATCH] D20914: [esan|cfrag] Compute the struct field access variance

[Mike Aizatsky via llvm-commits]

aizatsky added inline comments.

================
Comment at: lib/esan/cache_frag.cpp:44
@@ -41,3 +43,3 @@
   u64 Count;    // The total access count of the struct.
-  u32 Variance; // Variance score for the struct layout access.
+  u64 Variance; // Variance score for the struct layout access.
 };
----------------
Is variance a well-established term here? If not, we need to find something else.
Variance has a very strict meaning in my mind: "variance is the expectation of the squared deviation of a random variable from its mean".

================
Comment at: lib/esan/cache_frag.cpp:62
@@ +61,3 @@
+
+static u32 computeVarianceScore(u64 Val1, u64 Val2) {
+  u64 Ratio;
----------------
As far as I can see you compute this:

Log[t, v1 / (v2 + 1) / t]

Is this right?

This is equivalent to:

Log[t, v1/(v2+1)]-1

So lets:

a) document this formula
b) maybe get rid of this -1? Would simplify things.

================
Comment at: lib/esan/cache_frag.cpp:62
@@ +61,3 @@
+
+static u32 computeVarianceScore(u64 Val1, u64 Val2) {
+  u64 Ratio;
----------------
aizatsky wrote:
> As far as I can see you compute this:
> 
> Log[t, v1 / (v2 + 1) / t]
> 
> Is this right?
> 
> This is equivalent to:
> 
> Log[t, v1/(v2+1)]-1
> 
> So lets:
> 
> a) document this formula
> b) maybe get rid of this -1? Would simplify things.
If my assumption about formula is right, then how about logratio as a metric name?

================
Comment at: lib/esan/cache_frag.cpp:64
@@ +63,3 @@
+  u64 Ratio;
+  u32 Score = 0, Threshold = getFlags()->variance_threshold;
+  if (Val1 > Val2)
----------------
Let's add Threshold as a parameter.

================
Comment at: lib/esan/cache_frag.cpp:66
@@ +65,3 @@
+  if (Val1 > Val2)
+    Ratio = Val1 / (Val2 + 1) / Threshold;
+  else
----------------
Why + 1? Because of 0 values? 

What do you imagine computeVarianceScore(v1, 0, t) look like?

================
Comment at: lib/esan/cache_frag.cpp:108
@@ +107,3 @@
+    Handle->Count += Handle->Struct->FieldCounters[i];
+    Handle->Variance += computeVarianceScore(
+        Handle->Struct->FieldCounters[i - 1], Handle->Struct->FieldCounters[i]);
----------------
I'm not sure that adding log-ratios together is a good thing to do. Do you have any reasoning why you do it this way?


http://reviews.llvm.org/D20914