[ltt-dev] [PATCH] Fix passing factors in recursive function

[ltt-dev] [PATCH] Fix passing factors in recursive function

[Masoume Jabbarifar]

After finding the best path to the reference node, factors should
be converted in this path recursively.

Signed-off-by: Masoume Jabbarifar <masoume.jabbarifar at polymtl.ca>
---
 lttv/lttv/sync/factor_reduction_accuracy.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/lttv/lttv/sync/factor_reduction_accuracy.c b/lttv/lttv/sync/factor_reduction_accuracy.c
index a63dae7..ffb2d95 100644
--- a/lttv/lttv/sync/factor_reduction_accuracy.c
+++ b/lttv/lttv/sync/factor_reduction_accuracy.c
@@ -415,8 +415,14 @@ static void getFactors(AllFactors* const allFactors, unsigned int** const
 	unsigned int reference;
 	PairFactors** const pairFactors= allFactors->pairFactors;
 
-	reference= references[traceNum];
-
+	if (traceNum == references[traceNum])
+	{
+		reference= traceNum;
+	}
+	else
+	{
+		reference= predecessors[references[traceNum]][traceNum];
+	}
 	if (reference == traceNum)
 	{
 		factors->offset= 0.;
-- 
1.6.0.4

[ltt-dev] [PATCH 1/2] Use appropriate factor-pair when reducing

[Benjamin Poirier]

Correct a bug in one of the factor reduction functions.

To quote the README:
	Event analysis yields time correction factors between trace pairs. For
	groups of more than two traces, an extra step is needed to identify a
	reference trace and calculate correction factors for each trace relative
	to this reference.

After arranging the traces in a tree, getFactors() finds correction factors to
convert a trace's time to its reference's time. It does this by recursively
combining the factors between the trace and its parent with the factors
between the parent and the reference. Example:

A
 \_ B
 \_ C
     \_ D

With the following tree, calling getFactors() on D should combine the factors
between C and D with the result of calling getFactors() on C. (Calling
getFactors() on the reference, A, yields "identity" factors.)

The current implementation of getFactors() has a bug. When combining, it uses
the factors between a node and the reference instead of the factors between a
node and its parent. This is a problem on nodes with detph >= 2 and triggers
an assert(). This patch corrects the bug.

Signed-off-by: Benjamin Poirier <benjamin.poirier at polymtl.ca>
Reported-by: Masoume Jabbarifar <masoume.jabbarifar at polymtl.ca>
---

The fix suggested earlier by Masoume achieves the same end goal. This one
however doesn't add an extra conditionnal block. (A little simpler, perharps
because that's how the original author had meant to implement it in the first
place ;)

 lttv/lttv/sync/factor_reduction_accuracy.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/lttv/lttv/sync/factor_reduction_accuracy.c b/lttv/lttv/sync/factor_reduction_accuracy.c
index a63dae7..885c52c 100644
--- a/lttv/lttv/sync/factor_reduction_accuracy.c
+++ b/lttv/lttv/sync/factor_reduction_accuracy.c
@@ -425,30 +425,31 @@ static void getFactors(AllFactors* const allFactors, unsigned int** const
 	else
 	{
 		Factors previousVertexFactors;
+		unsigned int previousVertex= predecessors[reference][traceNum];
 
-		getFactors(allFactors, predecessors, references,
-			predecessors[reference][traceNum], &previousVertexFactors);
+		getFactors(allFactors, predecessors, references, previousVertex,
+			&previousVertexFactors);
 
 		/* Convert the time from traceNum to reference;
 		 * pairFactors[row][col] converts the time from col to row, invert the
 		 * factors as necessary */
 
-		if (pairFactors[reference][traceNum].approx != NULL)
+		if (pairFactors[previousVertex][traceNum].approx != NULL)
 		{
 			factors->offset= previousVertexFactors.drift *
-				pairFactors[reference][traceNum].approx->offset +
+				pairFactors[previousVertex][traceNum].approx->offset +
 				previousVertexFactors.offset;
 			factors->drift= previousVertexFactors.drift *
-				pairFactors[reference][traceNum].approx->drift;
+				pairFactors[previousVertex][traceNum].approx->drift;
 		}
-		else if (pairFactors[traceNum][reference].approx != NULL)
+		else if (pairFactors[traceNum][previousVertex].approx != NULL)
 		{
 			factors->offset= previousVertexFactors.drift * (-1. *
-				pairFactors[traceNum][reference].approx->offset /
-				pairFactors[traceNum][reference].approx->drift) +
+				pairFactors[traceNum][previousVertex].approx->offset /
+				pairFactors[traceNum][previousVertex].approx->drift) +
 				previousVertexFactors.offset;
 			factors->drift= previousVertexFactors.drift * (1. /
-				pairFactors[traceNum][reference].approx->drift);
+				pairFactors[traceNum][previousVertex].approx->drift);
 		}
 		else
 		{
-- 
1.7.1

[ltt-dev] [PATCH 2/2] Clean up style and redundancy

[Benjamin Poirier]

As the previous patch oultined, there was some repetition in getFactors() we
can do away with. While we're in the neighborhood, also wrap a > 80 col line.

Signed-off-by: Benjamin Poirier <benjamin.poirier at polymtl.ca>
---
 lttv/lttv/sync/factor_reduction_accuracy.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/lttv/lttv/sync/factor_reduction_accuracy.c b/lttv/lttv/sync/factor_reduction_accuracy.c
index 885c52c..ccb0d03 100644
--- a/lttv/lttv/sync/factor_reduction_accuracy.c
+++ b/lttv/lttv/sync/factor_reduction_accuracy.c
@@ -244,8 +244,8 @@ static GArray* finalizeReductionAccuracy(SyncState* const syncState,
 	g_array_set_size(factors, syncState->traceNb);
 	for (i= 0; i < syncState->traceNb; i++)
 	{
-		getFactors(allFactors, predecessors, references, i, &g_array_index(factors,
-				Factors, i));
+		getFactors(allFactors, predecessors, references, i,
+			&g_array_index(factors, Factors, i));
 	}
 
 	if (syncState->stats)
@@ -426,6 +426,8 @@ static void getFactors(AllFactors* const allFactors, unsigned int** const
 	{
 		Factors previousVertexFactors;
 		unsigned int previousVertex= predecessors[reference][traceNum];
+		PairFactors* factorsPT= &pairFactors[previousVertex][traceNum],
+			*factorsTP= &pairFactors[traceNum][previousVertex];
 
 		getFactors(allFactors, predecessors, references, previousVertex,
 			&previousVertexFactors);
@@ -434,22 +436,20 @@ static void getFactors(AllFactors* const allFactors, unsigned int** const
 		 * pairFactors[row][col] converts the time from col to row, invert the
 		 * factors as necessary */
 
-		if (pairFactors[previousVertex][traceNum].approx != NULL)
+		if (factorsPT->approx != NULL)
 		{
 			factors->offset= previousVertexFactors.drift *
-				pairFactors[previousVertex][traceNum].approx->offset +
-				previousVertexFactors.offset;
+				factorsPT->approx->offset + previousVertexFactors.offset;
 			factors->drift= previousVertexFactors.drift *
-				pairFactors[previousVertex][traceNum].approx->drift;
+				factorsPT->approx->drift;
 		}
-		else if (pairFactors[traceNum][previousVertex].approx != NULL)
+		else if (factorsTP->approx != NULL)
 		{
 			factors->offset= previousVertexFactors.drift * (-1. *
-				pairFactors[traceNum][previousVertex].approx->offset /
-				pairFactors[traceNum][previousVertex].approx->drift) +
+				factorsTP->approx->offset / factorsTP->approx->drift) +
 				previousVertexFactors.offset;
 			factors->drift= previousVertexFactors.drift * (1. /
-				pairFactors[traceNum][previousVertex].approx->drift);
+				factorsTP->approx->drift);
 		}
 		else
 		{
-- 
1.7.1

[ltt-dev] [PATCH] Fix passing factors in recursive function

[Mathieu Desnoyers]

* masoume.jabbarifar at polymtl.ca (masoume.jabbarifar at polymtl.ca) wrote:
> From: Masoume Jabbarifar <masoume.jabbarifar@polymtl.ca>
> 

Hi Masoume,

Please CC Benjamin when posting these patches, so we can get his feedback.

Also a description of the problem this is addressing would be welcome in
the patch header (before the "---" line).


Typical patch (the two first lines are provided by your mail client, you
have to provide the rest):

From: Your email
Subject: [PATCH] Fix passing factors in recursive function

Some detail about what the patch does.

Signed-off-by: Your email.


Thanks,

Mathieu

> ---
>  lttv/lttv/sync/factor_reduction_accuracy.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/lttv/lttv/sync/factor_reduction_accuracy.c b/lttv/lttv/sync/factor_reduction_accuracy.c
> index a63dae7..ffb2d95 100644
> --- a/lttv/lttv/sync/factor_reduction_accuracy.c
> +++ b/lttv/lttv/sync/factor_reduction_accuracy.c
> @@ -415,8 +415,14 @@ static void getFactors(AllFactors* const allFactors, unsigned int** const
>  	unsigned int reference;
>  	PairFactors** const pairFactors= allFactors->pairFactors;
>  
> -	reference= references[traceNum];
> -
> +	if (traceNum == references[traceNum])
> +	{
> +		reference= traceNum;
> +	}
> +	else
> +	{
> +		reference= predecessors[references[traceNum]][traceNum];
> +	}
>  	if (reference == traceNum)
>  	{
>  		factors->offset= 0.;
> -- 
> 1.6.0.4
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[ltt-dev] [PATCH] Fix passing factors in recursive function

[masoume.jabbarifar]

From: Masoume Jabbarifar <masoume.jabbarifar@polymtl.ca>

---
 lttv/lttv/sync/factor_reduction_accuracy.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/lttv/lttv/sync/factor_reduction_accuracy.c b/lttv/lttv/sync/factor_reduction_accuracy.c
index a63dae7..ffb2d95 100644
--- a/lttv/lttv/sync/factor_reduction_accuracy.c
+++ b/lttv/lttv/sync/factor_reduction_accuracy.c
@@ -415,8 +415,14 @@ static void getFactors(AllFactors* const allFactors, unsigned int** const
 	unsigned int reference;
 	PairFactors** const pairFactors= allFactors->pairFactors;
 
-	reference= references[traceNum];
-
+	if (traceNum == references[traceNum])
+	{
+		reference= traceNum;
+	}
+	else
+	{
+		reference= predecessors[references[traceNum]][traceNum];
+	}
 	if (reference == traceNum)
 	{
 		factors->offset= 0.;
-- 
1.6.0.4