* [ltt-dev] [PATCH 1/2] Use appropriate factor-pair when reducing
2010-09-14 20:52 [ltt-dev] [PATCH] Fix passing factors in recursive function Masoume Jabbarifar
@ 2010-09-20 1:01 ` Benjamin Poirier
2010-09-20 1:01 ` [ltt-dev] [PATCH 2/2] Clean up style and redundancy Benjamin Poirier
1 sibling, 0 replies; 3+ messages in thread
From: Benjamin Poirier @ 2010-09-20 1:01 UTC (permalink / raw)
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
^ permalink raw reply [flat|nested] 3+ messages in thread* [ltt-dev] [PATCH 2/2] Clean up style and redundancy
2010-09-14 20:52 [ltt-dev] [PATCH] Fix passing factors in recursive function Masoume Jabbarifar
2010-09-20 1:01 ` [ltt-dev] [PATCH 1/2] Use appropriate factor-pair when reducing Benjamin Poirier
@ 2010-09-20 1:01 ` Benjamin Poirier
1 sibling, 0 replies; 3+ messages in thread
From: Benjamin Poirier @ 2010-09-20 1:01 UTC (permalink / raw)
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
^ permalink raw reply [flat|nested] 3+ messages in thread