On 13/09/10 04:39 PM, Mathieu Desnoyers wrote:
* [email protected] ([email protected]) wrote:
From: Masoume Jabbarifar<[email protected]>
Can you document the change in the patch header ?
Also, please CC Benjamin on these changes. I'd like him to have a look
at them if he has time.
---
lttv/lttv/sync/data_structures.h | 2 +-
lttv/lttv/sync/factor_reduction_accuracy.c | 15 ++++++++++++---
lttv/lttv/sync/sync_chain_lttv.c | 18 ++++++++++++------
lttv/lttv/sync/sync_chain_unittest.c | 9 ++++++---
4 files changed, 31 insertions(+), 13 deletions(-)
diff --git a/lttv/lttv/sync/data_structures.h b/lttv/lttv/sync/data_structures.h
index 627286c..2206455 100644
--- a/lttv/lttv/sync/data_structures.h
+++ b/lttv/lttv/sync/data_structures.h
@@ -131,7 +131,7 @@ typedef struct
* synchronization */
typedef struct
{
- double drift, offset;
+ double drift, offset, accuracyToRefNode;
Hrm, I see that the sync code use a different coding style than the rest
of lttv. *sight* OK then.
Yeah, I know... I used my own style. Given that the coding style in lttv
is pretty loose in some places I didn't feel too guilty ;)
Concerning style, if this patch is to be consistent with the rest of the
sync code:
* put a space only to the right of assignment operators ('=', '+=', ...).
* put a space on each side of other operators
* put a space after commas (',')
Putting a space only to the right of the assignment operators has helped
me catch a few 'if (x= 0)' bugs. In any case, this patch in itself has
inconsistent styling. I've outlined the first occurrence where each rule
could apply in the patch.
For your information, the rest of lttv uses lower cases for variable
identifiers, and UpperCase for type identifiers. It also uses
lower_case_with_underscore() for function names.
} Factors;
// Correction factors between trace pairs, to be used for reduction
diff --git a/lttv/lttv/sync/factor_reduction_accuracy.c
b/lttv/lttv/sync/factor_reduction_accuracy.c
index a63dae7..bf0546b 100644
--- a/lttv/lttv/sync/factor_reduction_accuracy.c
+++ b/lttv/lttv/sync/factor_reduction_accuracy.c
@@ -415,12 +415,19 @@ 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;
No space around the '='
+ }
+ else
+ {
+ reference = predecessors[references[traceNum]][traceNum];
Spaces on both sides of the '='
+ }
if (reference == traceNum)
{
factors->offset= 0.;
factors->drift= 1.;
+ factors->accuracyToRefNode= 0.;
The spacing is right on that one.
The field should not be added to this struct however but in an array in
struct ReductionStatsAccuracy. The information that field holds is
specific to the "accuracy" reduction module and is only useful if the
user wants extra output about the innards of the synchronization
process. It's not useful in the general case where the user simply wants
to look at synchronized traces.
}
else
{
@@ -428,6 +435,8 @@ static void getFactors(AllFactors* const allFactors,
unsigned int** const
getFactors(allFactors, predecessors, references,
predecessors[reference][traceNum],&previousVertexFactors);
+ factors->accuracyToRefNode =
+ pairFactors[reference][traceNum].accuracy+
pairFactors[traceNum][reference].accuracy +
previousVertexFactors.accuracyToRefNode;
Please put a space on each side of the first '+'
/* Convert the time from traceNum to reference;
* pairFactors[row][col] converts the time from col to row,
invert the
@@ -452,7 +461,7 @@ static void getFactors(AllFactors* const allFactors,
unsigned int** const
}
else
{
- g_assert_not_reached();
+ //g_assert_not_reached();
Why commenting this ? This assertion was probably there for a reason.
Shutting up assertions without proper comments is usually a bad
practice, as assertions are our safety nets.
}
}
}
diff --git a/lttv/lttv/sync/sync_chain_lttv.c b/lttv/lttv/sync/sync_chain_lttv.c
index 95bef44..20ed111 100644
--- a/lttv/lttv/sync/sync_chain_lttv.c
+++ b/lttv/lttv/sync/sync_chain_lttv.c
@@ -218,6 +218,7 @@ bool syncTraceset(LttvTracesetContext* const
traceSetContext)
double minOffset, minDrift;
unsigned int refFreqTrace;
int retval;
+ double sumAccuracy;
if (!optionSync.present)
{
@@ -370,8 +371,6 @@ bool syncTraceset(LttvTracesetContext* const
traceSetContext)
t->drift * t->start_tsc + t->offset));
}
- g_array_free(factors, TRUE);
-
lttv_traceset_context_compute_time_span(traceSetContext,
&traceSetContext->time_span);
@@ -395,21 +394,28 @@ bool syncTraceset(LttvTracesetContext* const
traceSetContext)
if (!optionSyncNull.present&& optionSyncStats.present)
{
printStats(syncState);
-
+ sumAccuracy = 0;
= 0.;
printf("Resulting synchronization factors:\n");
for (i= 0; i< syncState->traceNb; i++)
{
LttTrace* t;
+ Factors* traceFactors;
t= traceSetContext->traces[i]->t;
-
- printf("\ttrace %u drift= %g offset= %g (%f) start time=
%ld.%09ld\n",
+ traceFactors=&g_array_index(factors, Factors, i);
+
+ printf("\ttrace %u drift= %g offset= %g (%f) start time=
%ld.%09ld accuracyToRefNode= %g\n",
Same thing regarding specificity to the "accuracy" reduction module. All
of this printing should be done in printReductionStatsAccuracy().
i, t->drift, t->offset, (double)
tsc_to_uint64(t->freq_scale,
t->start_freq, t->offset) /
NANOSECONDS_PER_SECOND,
t->start_time_from_tsc.tv_sec,
- t->start_time_from_tsc.tv_nsec);
+
t->start_time_from_tsc.tv_nsec,traceFactors->accuracyToRefNode);
+ sumAccuracy += traceFactors->accuracyToRefNode;
+
}
}
+ printf("Total accuracy (0.0 is the best): %g\n",sumAccuracy);
Please put a space after the comma
+
+ g_array_free(factors, TRUE);
syncState->processingModule->destroyProcessing(syncState);
if (syncState->matchingModule != NULL)
diff --git a/lttv/lttv/sync/sync_chain_unittest.c
b/lttv/lttv/sync/sync_chain_unittest.c
index 40302a0..d59eb09 100644
--- a/lttv/lttv/sync/sync_chain_unittest.c
+++ b/lttv/lttv/sync/sync_chain_unittest.c
@@ -114,6 +114,7 @@ int main(const int argc, char* const argv[])
GString* analysisModulesNames;
GString* reductionModulesNames;
unsigned int id;
+ double sumAccuracy;
AllFactors* allFactors;
/*
@@ -266,15 +267,17 @@ int main(const int argc, char* const argv[])
unsigned int i;
printStats(syncState);
-
+ sumAccuracy = 0;
= 0.;
Thanks,
Mathieu
printf("Resulting synchronization factors:\n");
for (i= 0; i< factors->len; i++)
{
Factors* traceFactors=&g_array_index(factors, Factors,
i);
- printf("\ttrace %u drift= %g offset= %g\n", i,
- traceFactors->drift, traceFactors->offset);
+ printf("\ttrace %u drift= %g offset= %g accuracyToRefNode=
%g\n", i,
+ traceFactors->drift, traceFactors->offset,
traceFactors->accuracyToRefNode);
+ sumAccuracy += traceFactors->accuracyToRefNode;
}
}
+ printf("Total accuracy (0.0 is the best): %g\n",sumAccuracy);
Printing it in printReductionStatsAccuracy() would avoid this duplication.
-Ben
// Destroy modules and clean up
syncState->processingModule->destroyProcessing(syncState);
--
1.6.0.4
_______________________________________________
ltt-dev mailing list
[email protected]
http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
_______________________________________________
ltt-dev mailing list
[email protected]
http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
