> On Feb 25, 2026, at 21:54, Chao Li <[email protected]> wrote:
> 
> 
> 
>> On Feb 25, 2026, at 21:10, Daniel Gustafsson <[email protected]> wrote:
>> 
>>> On 25 Feb 2026, at 13:41, Chao Li <[email protected]> wrote:
>>> 
>>>> On Feb 25, 2026, at 18:21, Daniel Gustafsson <[email protected]> wrote:
>>>> 
>>>>> On 25 Feb 2026, at 07:31, yangyz <[email protected]> wrote:
>>>> 
>>>>> 2.Performance Overhead
>>>>> In each iteration, the entire buffer of size LZ4_CHUNK_SZ (potentially 
>>>>> several megabytes) is zero-initialized. Since these memory blocks are 
>>>>> immediately overwritten by decompressed data, this zeroing operation 
>>>>> constitutes an unnecessary consumption of CPU resources.
>>>> 
>>>> When proposing a performance improvement it's important to provide some 
>>>> level
>>>> of benchmarks to show the improvement. Is removing this memset noticeable?
>>> 
>>> I don’t think this patch is about performance. Although removing the memset 
>>> might save a few CPU cycles, the real benefit seems to be cleanup and 
>>> consistency. The memset appears unnecessary, and similar functions don’t 
>>> use it, so I think this change mainly improves maintainability.
>> 
>> I would argue the opposite, clearing a buffer before passing it to an 
>> external
>> library function writing to it seems the right thing to do unless it can be
>> proven to regress performance too much.  Also, "appears unnecessary" doesn't
>> instill enough confidence to perform a change IMO.
>> 
>> --
>> Daniel Gustafsson
> 
> 
> As I pointed out earlier, ReadDataFromArchiveLZ4() has a very similar loop 
> that doesn’t zero out  the output buffer:
> ```
> while (readp < readend)
> {
>    size_t out_size = DEFAULT_IO_BUFFER_SIZE;
>    size_t read_size = readend - readp;
> 
>    status = LZ4F_decompress(ctx, outbuf, &out_size,
>                                                 readp, &read_size, &dec_opt);
>    if (LZ4F_isError(status))
>        pg_fatal("could not decompress: %s",
>              LZ4F_getErrorName(status));
> 
>    ahwrite(outbuf, 1, out_size, AH);
>    readp += read_size;
> }
> ```
> 
> Do you think we should add a memset there? There are a couple of more callers 
> of LZ4F_decompress that don’t zero out the output buffer.
> 
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
> 

Adding the original author to see if he still remember what was the intention 
of the memset.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/






Reply via email to