From: Yann Collet Date: Tue, 13 Nov 2018 21:05:39 +0000 (-0800) Subject: changed benchfn api X-Git-Tag: v1.3.8~47^2~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b830ccca5cc6a8dbe916742011a397ec384c7954;p=thirdparty%2Fzstd.git changed benchfn api to use structure for function parameters as it expresses much clearer than a long list of parameters, since each parameter can now be named. --- diff --git a/contrib/largeNbDicts/largeNbDicts.c b/contrib/largeNbDicts/largeNbDicts.c index 2605c6003..5a890cd90 100644 --- a/contrib/largeNbDicts/largeNbDicts.c +++ b/contrib/largeNbDicts/largeNbDicts.c @@ -538,16 +538,22 @@ static int benchMem(slice_collection_t dstBlocks, BMK_timedFnState_t* const benchState = BMK_createTimedFnState(total_time_ms, ms_per_round); decompressInstructions di = createDecompressInstructions(dictionaries); + BMK_benchParams_t const bp = { + .benchFn = decompress, + .benchPayload = &di, + .initFn = NULL, + .initPayload = NULL, + .errorFn = ZSTD_isError, + .blockCount = dstBlocks.nbSlices, + .srcBuffers = (const void* const*) srcBlocks.slicePtrs, + .srcSizes = srcBlocks.capacities, + .dstBuffers = dstBlocks.slicePtrs, + .dstCapacities = dstBlocks.capacities, + .blockResults = NULL + }; for (;;) { - BMK_runOutcome_t const outcome = BMK_benchTimedFn(benchState, - decompress, &di, - NULL, NULL, - ZSTD_isError, - dstBlocks.nbSlices, - (const void* const *)srcBlocks.slicePtrs, srcBlocks.capacities, - dstBlocks.slicePtrs, dstBlocks.capacities, - NULL); + BMK_runOutcome_t const outcome = BMK_benchTimedFn(benchState, bp); CONTROL(BMK_isSuccessful_runOutcome(outcome)); BMK_runTime_t const result = BMK_extract_runTime(outcome); diff --git a/programs/benchfn.c b/programs/benchfn.c index e2edce14e..f1be3394d 100644 --- a/programs/benchfn.c +++ b/programs/benchfn.c @@ -18,6 +18,7 @@ #include /* malloc, free */ #include /* memset */ #include /* fprintf, fopen */ +#undef NDEBUG /* assert must not be disabled */ #include /* assert */ #include "mem.h" @@ -44,7 +45,7 @@ # define DEBUG 0 #endif -#define DISPLAY(...) fprintf(stderr, __VA_ARGS__) +#define DISPLAY(...) fprintf(stderr, __VA_ARGS__) #define DEBUGOUTPUT(...) { if (DEBUG) DISPLAY(__VA_ARGS__); } /* error without displaying */ @@ -105,23 +106,16 @@ static BMK_runOutcome_t BMK_setValid_runTime(BMK_runTime_t runTime) /* can report result of benchFn for each block into blockResult. */ /* blockResult is optional, provide NULL if this information is not required */ /* note : time per loop can be reported as zero if run time < timer resolution */ -BMK_runOutcome_t BMK_benchFunction( - BMK_benchFn_t benchFn, void* benchPayload, - BMK_initFn_t initFn, void* initPayload, - BMK_errorFn_t errorFn, - size_t blockCount, - const void* const * srcBlockBuffers, const size_t* srcBlockSizes, - void* const * dstBlockBuffers, const size_t* dstBlockCapacities, - size_t* blockResults, - unsigned nbLoops) +BMK_runOutcome_t BMK_benchFunction(BMK_benchParams_t p, + unsigned nbLoops) { size_t dstSize = 0; nbLoops += !nbLoops; /* minimum nbLoops is 1 */ /* init */ { size_t i; - for(i = 0; i < blockCount; i++) { - memset(dstBlockBuffers[i], 0xE5, dstBlockCapacities[i]); /* warm up and erase result buffer */ + for(i = 0; i < p.blockCount; i++) { + memset(p.dstBuffers[i], 0xE5, p.dstCapacities[i]); /* warm up and erase result buffer */ } #if 0 /* based on testing these seem to lower accuracy of multiple calls of 1 nbLoops vs 1 call of multiple nbLoops @@ -135,20 +129,20 @@ BMK_runOutcome_t BMK_benchFunction( /* benchmark */ { UTIL_time_t const clockStart = UTIL_getTime(); unsigned loopNb, blockNb; - if (initFn != NULL) initFn(initPayload); + if (p.initFn != NULL) p.initFn(p.initPayload); for (loopNb = 0; loopNb < nbLoops; loopNb++) { - for (blockNb = 0; blockNb < blockCount; blockNb++) { - size_t const res = benchFn(srcBlockBuffers[blockNb], srcBlockSizes[blockNb], - dstBlockBuffers[blockNb], dstBlockCapacities[blockNb], - benchPayload); + for (blockNb = 0; blockNb < p.blockCount; blockNb++) { + size_t const res = p.benchFn(p.srcBuffers[blockNb], p.srcSizes[blockNb], + p.dstBuffers[blockNb], p.dstCapacities[blockNb], + p.benchPayload); if (loopNb == 0) { - if ((errorFn != NULL) && (errorFn(res))) { + if (p.blockResults != NULL) p.blockResults[blockNb] = res; + if ((p.errorFn != NULL) && (p.errorFn(res))) { RETURN_QUIET_ERROR(BMK_runOutcome_error(res), "Function benchmark failed on block %u (of size %u) with error %i", - blockNb, (U32)srcBlockBuffers[blockNb], (int)res); + blockNb, (U32)p.srcBuffers[blockNb], (int)res); } dstSize += res; - if (blockResults != NULL) blockResults[blockNb] = res; } } } /* for (loopNb = 0; loopNb < nbLoops; loopNb++) */ @@ -211,15 +205,8 @@ int BMK_isCompleted_TimedFn(const BMK_timedFnState_t* timedFnState) #define MINUSABLETIME (TIMELOOP_NANOSEC / 2) /* 0.5 seconds */ -BMK_runOutcome_t BMK_benchTimedFn( - BMK_timedFnState_t* cont, - BMK_benchFn_t benchFn, void* benchPayload, - BMK_initFn_t initFn, void* initPayload, - BMK_errorFn_t errorFn, - size_t blockCount, - const void* const* srcBlockBuffers, const size_t* srcBlockSizes, - void * const * dstBlockBuffers, const size_t * dstBlockCapacities, - size_t* blockResults) +BMK_runOutcome_t BMK_benchTimedFn(BMK_timedFnState_t* cont, + BMK_benchParams_t p) { U64 const runBudget_ns = cont->runBudget_ns; U64 const runTimeMin_ns = runBudget_ns / 2; @@ -237,14 +224,7 @@ BMK_runOutcome_t BMK_benchTimedFn( } /* reinitialize capacity */ - runResult = BMK_benchFunction(benchFn, benchPayload, - initFn, initPayload, - errorFn, - blockCount, - srcBlockBuffers, srcBlockSizes, - dstBlockBuffers, dstBlockCapacities, - blockResults, - cont->nbLoops); + runResult = BMK_benchFunction(p, cont->nbLoops); if(!BMK_isSuccessful_runOutcome(runResult)) { /* error : move out */ return runResult; diff --git a/programs/benchfn.h b/programs/benchfn.h index 608ddfa6e..3ca36e362 100644 --- a/programs/benchfn.h +++ b/programs/benchfn.h @@ -28,7 +28,7 @@ extern "C" { /* ==== Benchmark any function, iterated on a set of blocks ==== */ -/* BMK_runTime_t: valid result type */ +/* BMK_runTime_t: valid result return type */ typedef struct { unsigned long long nanoSecPerRun; /* time per iteration (over all blocks) */ @@ -36,9 +36,10 @@ typedef struct { } BMK_runTime_t; -/* type expressing the outcome of a benchmark run by BMK_benchFunction(). - * benchmark outcome might be valid or invalid. - * benchmark outcome can be invalid if and errorFn was provided. +/* BMK_runOutcome_t: + * type expressing the outcome of a benchmark run by BMK_benchFunction(), + * which can be either valid or invalid. + * benchmark outcome can be invalid if errorFn is provided. * BMK_runOutcome_t must be considered "opaque" : never access its members directly. * Instead, use its assigned methods : * BMK_isSuccessful_runOutcome, BMK_extract_runTime, BMK_extract_errorResult. @@ -51,48 +52,61 @@ typedef struct { } BMK_runOutcome_t; +/* prototypes for benchmarked functions */ typedef size_t (*BMK_benchFn_t)(const void* src, size_t srcSize, void* dst, size_t dstCapacity, void* customPayload); typedef size_t (*BMK_initFn_t)(void* initPayload); typedef unsigned (*BMK_errorFn_t)(size_t); +/* BMK_benchFunction() parameters are provided through following structure. + * This is preferable for readability, + * as the number of parameters required is pretty large. + * No initializer is provided, because it doesn't make sense to provide some "default" : + * all parameters should be specified by the caller */ +typedef struct { + BMK_benchFn_t benchFn; /* the function to benchmark, over the set of blocks */ + void* benchPayload; /* pass custom parameters to benchFn : + * (*benchFn)(srcBuffers[i], srcSizes[i], dstBuffers[i], dstCapacities[i], benchPayload) */ + BMK_initFn_t initFn; /* (*initFn)(initPayload) is run once per run, at the beginning. */ + void* initPayload; /* Both arguments can be NULL, in which case nothing is run. */ + BMK_errorFn_t errorFn; /* errorFn will check each return value of benchFn over each block, to determine if it failed or not. + * errorFn can be NULL, in which case no check is performed. + * errorFn must return 0 when benchFn was successful, and >= 1 if it detects an error. + * Execution is stopped as soon as an error is detected. + * the triggering return value can be retrieved using BMK_extract_errorResult(). */ + size_t blockCount; /* number of blocks to operate benchFn on. + * It's also the size of all array parameters : + * srcBuffers, srcSizes, dstBuffers, dstCapacities, blockResults */ + const void *const * srcBuffers; /* array of buffers to be operated on by benchFn */ + const size_t* srcSizes; /* array of the sizes of srcBuffers buffers */ + void *const * dstBuffers;/* array of buffers to be written into by benchFn */ + const size_t* dstCapacities; /* array of the capacities of dstBuffers buffers */ + size_t* blockResults; /* Optional: store the return value of benchFn for each block. Use NULL if this result is not requested. */ +} BMK_benchParams_t; + + /* BMK_benchFunction() : - * This function times the execution of 2 argument functions, benchFn and initFn */ - -/* benchFn - (*benchFn)(srcBuffers[i], srcSizes[i], dstBuffers[i], dstCapacities[i], benchPayload) - * is run nbLoops times - * initFn - (*initFn)(initPayload) is run once per run, at the beginning. - * This argument can be NULL, in which case nothing is run. - * errorFn - is a function run on each return value of benchFn. - * Argument errorFn can be NULL, in which case nothing is run. - * Otherwise, it must return 0 when benchFn was successful, and >= 1 if it detects an error. - * Execution is stopped as soon as an error is detected, - * the triggering return value can then be retrieved with BMK_extract_errorResult(). - * blockCount - number of blocks. Size of all array parameters : srcBuffers, srcSizes, dstBuffers, dstCapacities, blockResults - * srcBuffers - an array of buffers to be operated on by benchFn - * srcSizes - an array of the sizes of above buffers - * dstBuffers - an array of buffers to be written into by benchFn - * dstCapacities - an array of the capacities of above buffers - * blockResults - Optional: store the return value of benchFn for each block. Use NULL if this result is not requested. - * nbLoops - defines number of times benchFn is run. Minimum value is 1. A 0 is interpreted as a 1. - * @return: a variant, which express either an error, or can generate a valid BMK_runTime_t result. - * Use BMK_isSuccessful_runOutcome() to check if function was successful. + * This function benchmarks benchFn and initFn, providing a result. + * + * params : see description of BMK_benchParams_t above. + * nbLoops: defines number of times benchFn is run over the full set of blocks. + * Minimum value is 1. A 0 is interpreted as a 1. + * + * @return: can express either an error or a successful result. + * Use BMK_isSuccessful_runOutcome() to check if benchmark was successful. * If yes, extract the result with BMK_extract_runTime(), * it will contain : * .sumOfReturn : the sum of all return values of benchFn through all of blocks * .nanoSecPerRun : time per run of benchFn + (time for initFn / nbLoops) * .sumOfReturn is generally intended for functions which return a # of bytes written into dstBuffer, * in which case, this value will be the total amount of bytes written into dstBuffer. + * + * blockResults : when provided (!= NULL), and when benchmark is successful, + * params.blockResults contains all return values of `benchFn` over all blocks. + * when provided (!= NULL), and when benchmark failed, + * params.blockResults contains return values of `benchFn` over all blocks preceding and including the failed block. */ -BMK_runOutcome_t BMK_benchFunction( - BMK_benchFn_t benchFn, void* benchPayload, - BMK_initFn_t initFn, void* initPayload, - BMK_errorFn_t errorFn, - size_t blockCount, - const void *const * srcBuffers, const size_t* srcSizes, - void *const * dstBuffers, const size_t* dstCapacities, - size_t* blockResults, - unsigned nbLoops); +BMK_runOutcome_t BMK_benchFunction(BMK_benchParams_t params, unsigned nbLoops); @@ -106,7 +120,7 @@ int BMK_isSuccessful_runOutcome(BMK_runOutcome_t outcome); BMK_runTime_t BMK_extract_runTime(BMK_runOutcome_t outcome); /* when benchmark failed, it means one invocation of `benchFn` failed. - * The failure was detected by `errorFn`, operating on return value of `benchFn`. + * The failure was detected by `errorFn`, operating on return values of `benchFn`. * Returns the faulty return value. * note : this function will abort() program execution if benchmark did not failed. * always check if benchmark failed first ! @@ -128,15 +142,8 @@ typedef struct BMK_timedFnState_s BMK_timedFnState_t; * call BMK_benchTimedFn() repetitively, each measurement is supposed to last about run_ms * Check if total time budget is spent or exceeded, using BMK_isCompleted_TimedFn() */ -BMK_runOutcome_t BMK_benchTimedFn( - BMK_timedFnState_t* timedFnState, - BMK_benchFn_t benchFn, void* benchPayload, - BMK_initFn_t initFn, void* initPayload, - BMK_errorFn_t errorFn, - size_t blockCount, - const void *const * srcBlockBuffers, const size_t* srcBlockSizes, - void *const * dstBlockBuffers, const size_t* dstBlockCapacities, - size_t* blockResults); +BMK_runOutcome_t BMK_benchTimedFn(BMK_timedFnState_t* timedFnState, + BMK_benchParams_t params); /* Tells if duration of all benchmark runs has exceeded total_ms */ diff --git a/programs/benchzstd.c b/programs/benchzstd.c index 6738fe952..7f2da9318 100644 --- a/programs/benchzstd.c +++ b/programs/benchzstd.c @@ -403,14 +403,41 @@ BMK_benchMemAdvancedNoAlloc( U32 markNb = 0; int compressionCompleted = (adv->mode == BMK_decodeOnly); int decompressionCompleted = (adv->mode == BMK_compressOnly); + BMK_benchParams_t cbp, dbp; BMK_initCCtxArgs cctxprep; BMK_initDCtxArgs dctxprep; + + cbp.benchFn = local_defaultCompress; + cbp.benchPayload = cctx; + cbp.initFn = local_initCCtx; + cbp.initPayload = &cctxprep; + cbp.errorFn = ZSTD_isError; + cbp.blockCount = nbBlocks; + cbp.srcBuffers = srcPtrs; + cbp.srcSizes = srcSizes; + cbp.dstBuffers = cPtrs; + cbp.dstCapacities = cCapacities; + cbp.blockResults = cSizes; + cctxprep.cctx = cctx; cctxprep.dictBuffer = dictBuffer; cctxprep.dictBufferSize = dictBufferSize; cctxprep.cLevel = cLevel; cctxprep.comprParams = comprParams; cctxprep.adv = adv; + + dbp.benchFn = local_defaultDecompress; + dbp.benchPayload = dctx; + dbp.initFn = local_initDCtx; + dbp.initPayload = &dctxprep; + dbp.errorFn = ZSTD_isError; + dbp.blockCount = nbBlocks; + dbp.srcBuffers = (const void* const *) cPtrs; + dbp.srcSizes = cSizes; + dbp.dstBuffers = resPtrs; + dbp.dstCapacities = resSizes; + dbp.blockResults = NULL; + dctxprep.dctx = dctx; dctxprep.dictBuffer = dictBuffer; dctxprep.dictBufferSize = dictBufferSize; @@ -420,15 +447,7 @@ BMK_benchMemAdvancedNoAlloc( while (!(compressionCompleted && decompressionCompleted)) { if (!compressionCompleted) { - BMK_runOutcome_t const cOutcome = - BMK_benchTimedFn( timeStateCompress, - &local_defaultCompress, cctx, - &local_initCCtx, &cctxprep, - ZSTD_isError, - nbBlocks, - srcPtrs, srcSizes, - cPtrs, cCapacities, - cSizes); + BMK_runOutcome_t const cOutcome = BMK_benchTimedFn( timeStateCompress, cbp); if (!BMK_isSuccessful_runOutcome(cOutcome)) { return BMK_benchOutcome_error(); @@ -455,15 +474,7 @@ BMK_benchMemAdvancedNoAlloc( } if(!decompressionCompleted) { - BMK_runOutcome_t const dOutcome = - BMK_benchTimedFn(timeStateDecompress, - &local_defaultDecompress, dctx, - &local_initDCtx, &dctxprep, - ZSTD_isError, - nbBlocks, - (const void *const *)cPtrs, cSizes, - resPtrs, resSizes, - NULL); + BMK_runOutcome_t const dOutcome = BMK_benchTimedFn(timeStateDecompress, dbp); if(!BMK_isSuccessful_runOutcome(dOutcome)) { return BMK_benchOutcome_error(); diff --git a/tests/fullbench.c b/tests/fullbench.c index bd4b116d9..30f07b6cd 100644 --- a/tests/fullbench.c +++ b/tests/fullbench.c @@ -515,21 +515,26 @@ static size_t benchMem(U32 benchNb, /* benchmark loop */ { BMK_timedFnState_t* const tfs = BMK_createTimedFnState(g_nbIterations * 1000, 1000); + BMK_benchParams_t bp; BMK_runTime_t bestResult; bestResult.sumOfReturn = 0; bestResult.nanoSecPerRun = (unsigned long long)(-1LL); assert(tfs != NULL); + + bp.benchFn = benchFunction; + bp.benchPayload = buff2; + bp.initFn = NULL; + bp.initPayload = NULL; + bp.errorFn = ZSTD_isError; + bp.blockCount = 1; + bp.srcBuffers = &src; + bp.srcSizes = &srcSize; + bp.dstBuffers = (void* const*) &dstBuff; + bp.dstCapacities = &dstBuffSize; + bp.blockResults = NULL; + for (;;) { - void* const dstBuffv = dstBuff; - BMK_runOutcome_t const bOutcome = - BMK_benchTimedFn( tfs, - benchFunction, buff2, - NULL, NULL, /* initFn */ - ZSTD_isError, - 1, /* blockCount */ - &src, &srcSize, - &dstBuffv, &dstBuffSize, - NULL); + BMK_runOutcome_t const bOutcome = BMK_benchTimedFn( tfs, bp); if (!BMK_isSuccessful_runOutcome(bOutcome)) { DISPLAY("ERROR benchmarking function ! ! \n"); diff --git a/tests/paramgrill.c b/tests/paramgrill.c index 1b33c086d..cd272d9af 100644 --- a/tests/paramgrill.c +++ b/tests/paramgrill.c @@ -1398,7 +1398,8 @@ static void randomConstrainedParams(paramValues_t* pc, const memoTable_t* memoTa /* nbSeconds used in same way as in BMK_advancedParams_t */ /* if in decodeOnly, then srcPtr's will be compressed blocks, and uncompressedBlocks will be written to dstPtrs */ /* dictionary nullable, nothing else though. */ -/* note : it would be better if this function was in bench.c, sharing code with benchMemAdvanced(), since it's technically a part of it */ +/* note : it would be a lot better if this function was present in benchzstd.c, + * sharing code with benchMemAdvanced(), since it's technically a part of it */ static BMK_benchOutcome_t BMK_benchMemInvertible( buffers_t buf, contexts_t ctx, int cLevel, const paramValues_t* comprParams, @@ -1439,13 +1440,40 @@ BMK_benchMemInvertible( buffers_t buf, contexts_t ctx, int decompressionCompleted = (mode == BMK_compressOnly); BMK_timedFnState_t* timeStateCompress = BMK_createTimedFnState(nbSeconds * 1000, 1000); BMK_timedFnState_t* timeStateDecompress = BMK_createTimedFnState(nbSeconds * 1000, 1000); + BMK_benchParams_t cbp, dbp; BMK_initCCtxArgs cctxprep; BMK_initDCtxArgs dctxprep; + + cbp.benchFn = local_defaultCompress; + cbp.benchPayload = cctx; + cbp.initFn = local_initCCtx; + cbp.initPayload = &cctxprep; + cbp.errorFn = ZSTD_isError; + cbp.blockCount = nbBlocks; + cbp.srcBuffers = srcPtrs; + cbp.srcSizes = srcSizes; + cbp.dstBuffers = dstPtrs; + cbp.dstCapacities = dstCapacities; + cbp.blockResults = dstSizes; + cctxprep.cctx = cctx; cctxprep.dictBuffer = dictBuffer; cctxprep.dictBufferSize = dictBufferSize; cctxprep.cLevel = cLevel; cctxprep.comprParams = comprParams; + + dbp.benchFn = local_defaultDecompress; + dbp.benchPayload = dctx; + dbp.initFn = local_initDCtx; + dbp.initPayload = &dctxprep; + dbp.errorFn = ZSTD_isError; + dbp.blockCount = nbBlocks; + dbp.srcBuffers = (const void* const *) dstPtrs; + dbp.srcSizes = dstCapacities; + dbp.dstBuffers = resPtrs; + dbp.dstCapacities = resSizes; + dbp.blockResults = NULL; + dctxprep.dctx = dctx; dctxprep.dictBuffer = dictBuffer; dctxprep.dictBufferSize = dictBufferSize; @@ -1453,14 +1481,7 @@ BMK_benchMemInvertible( buffers_t buf, contexts_t ctx, assert(timeStateCompress != NULL); assert(timeStateDecompress != NULL); while(!compressionCompleted) { - BMK_runOutcome_t const cOutcome = BMK_benchTimedFn(timeStateCompress, - &local_defaultCompress, cctx, - &local_initCCtx, &cctxprep, - ZSTD_isError, - nbBlocks, - srcPtrs, srcSizes, - dstPtrs, dstCapacities, - dstSizes); + BMK_runOutcome_t const cOutcome = BMK_benchTimedFn(timeStateCompress, cbp); if (!BMK_isSuccessful_runOutcome(cOutcome)) { BMK_benchOutcome_t bOut; @@ -1478,14 +1499,7 @@ BMK_benchMemInvertible( buffers_t buf, contexts_t ctx, } while (!decompressionCompleted) { - BMK_runOutcome_t const dOutcome = BMK_benchTimedFn(timeStateDecompress, - &local_defaultDecompress, dctx, - &local_initDCtx, &dctxprep, - ZSTD_isError, - nbBlocks, - (const void* const*)dstPtrs, dstSizes, - resPtrs, resSizes, - NULL); + BMK_runOutcome_t const dOutcome = BMK_benchTimedFn(timeStateDecompress, dbp); if (!BMK_isSuccessful_runOutcome(dOutcome)) { BMK_benchOutcome_t bOut;