Refactor ZSTD_estimateCCtxSize_usingCCtxParams_internal()

[embg]

ZSTD_estimateCCtxSize_usingCCtxParams_internal() currently takes a long list of parameters. Some (all?) of the parameters are extracted from external ZSTD_CCtx_params objects and passed in one by one. It would be cleaner to reduce the number of parameters by directly passing in a ZSTD_CCtx_params const*.

This should be an easy refactor. Code pointer:

zstd/lib/compress/zstd_compress.c

Lines 1488 to 1495 in 90597d7

	static size_t ZSTD_estimateCCtxSize_usingCCtxParams_internal(
	const ZSTD_compressionParameters* cParams,
	const ldmParams_t* ldmParams,
	const int isStatic,
	const ZSTD_paramSwitch_e useRowMatchFinder,
	const size_t buffInSize,
	const size_t buffOutSize,
	const U64 pledgedSrcSize)

[benjamintli]

Can I take a crack at this?

[embg]

@benjamintli Sure, feel free to reach out with any questions

[benjamintli]

I noticed there's already a function here: https://github.com/facebook/zstd/blob/dev/lib/compress/zstd_compress.c#L1592 that seems to take in the params and parse the values out before passing to ZSTD_estimateCCtxSize_usingCCtxParams_internal. The function seems to do some call some helpers to determine arguments (e.g. compression params).

Wondering if it's still worth it or makes sense to refactor ZSTD_estimateCCtxSize_usingCCtxParams_internal?

[Cyan4973]

Do you mean that, instead of invoking ZSTD_estimateCCtxSize_usingCCtxParams_internal(), it would make better sense to invoke ZSTD_estimateCCtxSize_usingCCtxParams() ?

It depends if the scope of *_internal() is the same as _usingCCtxParams(), or if it can do more, and this "more" is exactly what's needed in the place where it's invoked.

At a quick glance, ZSTD_estimateCCtxSize_usingCCtxParams() is indeed reduced to accessing a portion of the scope of *_internal(), with multiple parameters being statically set.

[embg]

Ya, the problem here is that some of the parameters to ZSTD_estimateCCtxSize_usingCCtxParams_internal() aren't part of ZSTD_CCtx_params. For example, the isStatic parameter.

So we'll need to keep the ZSTD_estimateCCtxSize_usingCCtxParams_internal() function. Let's identify which of its parameters are part of the ZSTD_CCtx_params struct, and eliminate those while keeping others like isStatic.

The signature of ZSTD_estimateCCtxSize_usingCCtxParams_internal() should end up looking like this:

static size_t ZSTD_estimateCCtxSize_usingCCtxParams_internal(
        const ZSTD_CCtx_params* cctxParams,
        const int isStatic,
        // Add any other parameters here which can't be accessed through cctxParams
);