Hi Star, Thanks for your feedback. Will address your comments in patch v3.
Thanks, Cinnamon Shia -----Original Message----- From: Zeng, Star [mailto:star.z...@intel.com] Sent: Tuesday, November 3, 2015 10:06 PM To: Shia, Cinnamon; edk2-devel@lists.01.org Subject: Re: [edk2] [PATCH v2 2/2] ShellPkg/UefiDpLib: Support dumping cumulative data On 2015/11/3 21:26, Cinnamon Shia wrote: > Add a new option -c to dump cumulative data. > For example: > shell> dp -c > ==[ Cumulative ]======== > (Times in microsec.) Cumulative Average Shortest Longest > Name Count Duration Duration Duration Duration > LoadImage: 200 1000000 7000 0 100000 > StartImage: 200 20000000 90000 0 7000000 > DB:Start: 200 20000000 100000 0 9000000 > DB:Support: 200000 100000 0 0 7000 > > shell> dp -c DXE > ==[ Cumulative ]======== > (Times in microsec.) Cumulative Average Shortest Longest > Name Count Duration Duration Duration Duration > LoadImage: 200 1000000 7000 0 100000 > StartImage: 200 20000000 90000 0 7000000 > DB:Start: 200 20000000 100000 0 9000000 > DB:Support: 200000 100000 0 0 7000 > DXE 1 30000000 30000000 0 30000000 > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Cinnamon Shia <cinnamon.s...@hpe.com> > --- > ShellPkg/Library/UefiDpLib/Dp.c | 33 +++++++++++++++++-- > ShellPkg/Library/UefiDpLib/DpInternal.h | 9 +++-- > ShellPkg/Library/UefiDpLib/DpTrace.c | 55 > ++++++++++++++++++++++++++++--- > ShellPkg/Library/UefiDpLib/UefiDpLib.uni | Bin 17466 -> 18146 bytes > 4 files changed, 87 insertions(+), 10 deletions(-) If you accept my one minor comment below. Reviewed-by: Star Zeng <star.z...@intel.com> > > diff --git a/ShellPkg/Library/UefiDpLib/Dp.c > b/ShellPkg/Library/UefiDpLib/Dp.c index 62a4e7b..4d109d0 100644 > --- a/ShellPkg/Library/UefiDpLib/Dp.c > +++ b/ShellPkg/Library/UefiDpLib/Dp.c > @@ -79,6 +79,7 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] = { > #endif // PROFILING_IMPLEMENTED > {L"-x", TypeFlag}, // -x eXclude Cumulative Items > {L"-i", TypeFlag}, // -i Display Identifier > + {L"-c", TypeValue}, // -c Display cumulative data. > {L"-n", TypeValue}, // -n # Number of records to display for A and R > {L"-t", TypeValue}, // -t # Threshold of interest > {NULL, TypeMax} > @@ -164,6 +165,9 @@ ShellCommandRunDp ( > BOOLEAN TraceMode; > BOOLEAN ProfileMode; > BOOLEAN ExcludeMode; > + BOOLEAN CumulativeMode; > + CONST CHAR16 *CustomCumulativeToken; > + PERF_CUM_DATA *CustomCumulativeData; > > StringPtr = NULL; > SummaryMode = FALSE; > @@ -173,6 +177,8 @@ ShellCommandRunDp ( > TraceMode = FALSE; > ProfileMode = FALSE; > ExcludeMode = FALSE; > + CumulativeMode = FALSE; > + CustomCumulativeData = NULL; > > // Get DP's entry time as soon as possible. > // This is used as the Shell-Phase end time. > @@ -210,6 +216,7 @@ ShellCommandRunDp ( > #endif // PROFILING_IMPLEMENTED > ExcludeMode = ShellCommandLineGetFlag (ParamPackage, L"-x"); > mShowId = ShellCommandLineGetFlag (ParamPackage, L"-i"); > + CumulativeMode = ShellCommandLineGetFlag (ParamPackage, L"-c"); > > // Options with Values > CmdLineArg = ShellCommandLineGetValue (ParamPackage, L"-n"); @@ > -244,6 +251,20 @@ ShellCommandRunDp ( > InitCumulativeData (); > > // > + // Init the custom cumulative data. > + // > + CustomCumulativeToken = ShellCommandLineGetValue (ParamPackage, > + L"-c"); if (CustomCumulativeToken != NULL) { > + CustomCumulativeData = AllocateZeroPool (sizeof (PERF_CUM_DATA)); > + CustomCumulativeData->MinDur = 0; > + CustomCumulativeData->MaxDur = 0; > + CustomCumulativeData->Count = 0; > + CustomCumulativeData->Duration = 0; > + CustomCumulativeData->Name = AllocateZeroPool (StrLen > (CustomCumulativeToken) + 1); > + UnicodeStrToAsciiStr (CustomCumulativeToken, > + CustomCumulativeData->Name); } > + > + // > // Timer specific processing > // > // Get the Performance counter characteristics: > @@ -302,8 +323,10 @@ ShellCommandRunDp ( > **** !T && P := (2) Only Profile records are displayed > **** T && P := (3) Same as Default, both are displayed > > ********************************************************************** > ******/ > - GatherStatistics(); > - if (AllMode) { > + GatherStatistics (CustomCumulativeData); if (CumulativeMode) { > + ProcessCumulative (CustomCumulativeData); } else if (AllMode) { > if (TraceMode) { > DumpAllTrace( Number2Display, ExcludeMode); > } > @@ -326,7 +349,7 @@ ShellCommandRunDp ( > if ( ! EFI_ERROR( Status)) { > ProcessPeims (); > ProcessGlobal (); > - ProcessCumulative (); > + ProcessCumulative (NULL); > } > } > } > @@ -339,6 +362,10 @@ ShellCommandRunDp ( > } > > SHELL_FREE_NON_NULL (StringPtr); > + if (CustomCumulativeData != NULL) { > + SHELL_FREE_NON_NULL (CustomCumulativeData->Name); } > + SHELL_FREE_NON_NULL (CustomCumulativeData); > > return SHELL_SUCCESS; > } > diff --git a/ShellPkg/Library/UefiDpLib/DpInternal.h > b/ShellPkg/Library/UefiDpLib/DpInternal.h > index 9b8163a..b51f0d8 100644 > --- a/ShellPkg/Library/UefiDpLib/DpInternal.h > +++ b/ShellPkg/Library/UefiDpLib/DpInternal.h > @@ -172,10 +172,13 @@ GetCumulativeItem( > > @post The SummaryData and CumData structures contain statistics for the > current performance logs. > + > + @param[in] CustomCumulativeData The pointer to the custom cumulative > data. > + > **/ > VOID > GatherStatistics( > - VOID > + IN PERF_CUM_DATA *CustomCumulativeData OPTIONAL > ); > > /** > @@ -283,11 +286,13 @@ ProcessGlobal( > For each record with a Token listed in the CumData array:<BR> > - Update the instance count and the total, minimum, and maximum > durations. > Finally, print the gathered cumulative statistics. > + > + @param[in] CustomCumulativeData The pointer to the custom cumulative > data. > > **/ > VOID > ProcessCumulative( > - VOID > + IN PERF_CUM_DATA *CustomCumulativeData OPTIONAL > ); > > /** > diff --git a/ShellPkg/Library/UefiDpLib/DpTrace.c > b/ShellPkg/Library/UefiDpLib/DpTrace.c > index cf8200c..cc29cf2 100644 > --- a/ShellPkg/Library/UefiDpLib/DpTrace.c > +++ b/ShellPkg/Library/UefiDpLib/DpTrace.c > @@ -43,11 +43,14 @@ > > @post The SummaryData and CumData structures contain statistics for the > current performance logs. > + > + @param[in] CustomCumulativeData A pointer to the cumtom cumulative > data. > + @param[in] should be @param[in, out] to match with the parameter in the function header? > **/ > VOID > GatherStatistics( > - VOID > -) > + IN OUT PERF_CUM_DATA *CustomCumulativeData OPTIONAL > + ) > { > MEASUREMENT_RECORD Measurement; > UINT64 Duration; > @@ -99,6 +102,20 @@ GatherStatistics( > CumData[TIndex].MaxDur = Duration; > } > } > + > + // > + // Collect the data for custom cumulative data. > + // > + if ((CustomCumulativeData != NULL) && (AsciiStrCmp (Measurement.Token, > CustomCumulativeData->Name) == 0)) { > + CustomCumulativeData->Duration += Duration; > + CustomCumulativeData->Count++; > + if (Duration < CustomCumulativeData->MinDur) { > + CustomCumulativeData->MinDur = Duration; > + } > + if (Duration > CustomCumulativeData->MaxDur) { > + CustomCumulativeData->MaxDur = Duration; > + } > + } > } > } > > @@ -782,12 +799,14 @@ ProcessGlobal( > For each record with a Token listed in the CumData array:<BR> > - Update the instance count and the total, minimum, and maximum > durations. > Finally, print the gathered cumulative statistics. > - > + > + @param[in] CustomCumulativeData A pointer to the cumtom cumulative > data. > + > **/ > VOID > ProcessCumulative( > - VOID > -) > + IN PERF_CUM_DATA *CustomCumulativeData OPTIONAL > + ) > { > UINT64 AvgDur; // the computed average duration > UINT64 Dur; > @@ -826,4 +845,30 @@ ProcessCumulative( > ); > } > } > + > + // > + // Print the custom cumulative data. > + // > + if (CustomCumulativeData != NULL) { > + if (CustomCumulativeData->Count != 0) { > + AvgDur = DivU64x32 (CustomCumulativeData->Duration, > CustomCumulativeData->Count); > + AvgDur = DurationInMicroSeconds (AvgDur); > + Dur = DurationInMicroSeconds (CustomCumulativeData->Duration); > + MaxDur = DurationInMicroSeconds (CustomCumulativeData->MaxDur); > + MinDur = DurationInMicroSeconds (CustomCumulativeData->MinDur); > + } else { > + AvgDur = 0; > + Dur = 0; > + MaxDur = 0; > + MinDur = 0; > + } > + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DP_CUMULATIVE_STATS), > gDpHiiHandle, > + CustomCumulativeData->Name, > + CustomCumulativeData->Count, > + Dur, > + AvgDur, > + MinDur, > + MaxDur > + ); > + } > } > diff --git a/ShellPkg/Library/UefiDpLib/UefiDpLib.uni > b/ShellPkg/Library/UefiDpLib/UefiDpLib.uni > index > 1e5c26ac944e3f626c97c79ac9bb1c4196c70ed3..5bcb4964523aa7839694bc430f57 > ad27e8f63047 100644 GIT binary patch delta 437 > zcmZvY%}&BV6ot>kz=Fh81IBbSc8>}EM5qg+kpz)o6WIZaNgW8GMN$D@!HtWiPv9G9 > zy5nu+NjTFG(MTq}(|hlC&$*f9n^?Vz&!&i_b8!xd1~gs~FwS@+JSg}WGe&q~UdM!W > z&1}kVgcs;IwD(%@;twlrilA!w#JYr@Ii`vC(NDi>!(7(r-~snkX%i|#CP!7P8xbp0 > zU1LDS32)=1_-Ae(bGjJ0aFd0s1nkd9FGx#rCR6Bfryl1V*7i|=lc>CX?XkMc!6^UN > z+AeN(Fq3ZbeOXKf`O;wp$5d`{7e&uIPAd$56ZT0)JZ9TOj60Zv(~P;r4P0!=9~b-V > RPk+vrp3>2%xv99S<<^p%SkM3f > > delta 24 > gcmaFV%ebq9af27*<|~Xj7Mmlil2|t%a8zOi0D&(FVgLXD > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
