Replace ziplist with listpack in quicklist

[sundb]

Part three of implementing #8702, following #8887 and #9366 .

Description of the feature

1. Replace the ziplist container of quicklist with listpack.
2. Convert existing quicklist ziplists on RDB loading time. an O(n) operation.

Interface changes

1. New list-max-listpack-size config is an alias for list-max-ziplist-size.
2. Replace debug ziplist command with debug listpack.

Internal changes

1. Add lpMerge to merge two listpacks . (same as ziplistMerge)
2. Add lpRepr to print info of listpack which is used in debugCommand and quicklistRepr. (same as ziplistRepr)
3. Replace QUICKLIST_NODE_CONTAINER_ZIPLIST with QUICKLIST_NODE_CONTAINER_PACKED(following Add support for list type to store elements larger than 4GB #9357 ).
   It represent that a quicklistNode is a packed node, as opposed to a plain node.
4. Remove createZiplistObject method, which is never used.
5. Calculate listpack entry size using overhead overestimation in quicklistAllowInsert.
   We prefer an overestimation, which would at worse lead to a few bytes below the lowest limit of 4k.

Improvements

1. Calling lpShrinkToFit after converting Ziplist to listpack, which was missed at Replace all usage of ziplist with listpack for t_zset #9366.
2. Optimize quicklistAppendPlainNode to avoid memcpy data.

Bugfix

1. Fix crash in quicklistRepr when ziplist is compressed, introduced from Replace all usage of ziplist with listpack for t_zset #9366.

Test

1. Add unittest for lpMerge.
2. Modify the old quicklist ziplist corrupt dump test.

TODO

• Comment typos.
• Benchmark.

[oranagra]

@sundb thank you,
i guess we wanna see some benchmarks.
rdb loading to make sure we know the price
and commands to make sure there's no big regression.

i see rdb file format conversion test wasn't added, it is because the one added in #9357 covers it?

p.s. edited your top comment a bit (no need for backwards compatibility for DEBUG command).

[sundb]

@sundb thank you, i guess we wanna see some benchmarks. rdb loading to make sure we know the price and commands to make sure there's no big regression.

It's still on the way.

i see rdb file format conversion test wasn't added, it is because the one added in #9357 covers it?

Yes.

[sundb]

From the results, we can see that the performance of l/rpush and l/rpop operations is basically unchanged, but the performance of lrange for quicklist listpack is a little bit degraded.
The main reason is that the performance of lpNext/lpGet is not as good as ziplistNext/ziplistGet, which use a lot of ifs to fetch data.

  1000000 requests
  50 parallel clients
  3 bytes payload
  keep alive: 1
  host configuration "save": 
  host configuration "appendonly": no
  multi-thread: no

quicklist info

count: 1000000
len: 612
fill: -2
compress: 0
listpack items per node: 8182

command	quicklist listpack(rps)	quicklist ziplist(rps)	percentage(positive: performance enhancement, negative: performance degradation)
LPUSH	106929 rps, p50=0.223	103535.75 rps, p50=0.223	3.2%
LRANGE_100	65737.58 rps, p50=0.375	67333.27 rps, p50=0.375	-2.3%
LRANGE_300	30900.44 rps, p50=0.839	31346.00 rps, p50=0.823	-1.4%
LRANGE_500	22126.59 rps, p50=1.175	22689.63 rps, p50=1.159	-1.4%
LRANGE_600	19443.90 rps, p50=1.343	19690.08 rps, p50=1.327	-1.2%
RPUSH	109075.04 rps, p50=0.223	108283.71 rps, p50=0.223	0.7%
LPOP	108636.61 rps, p50=0.223	110778.77 rps, p50=0.223	1.9%
RPOP	109757.44 rps, p50=0.223	109206.08 rps, p50=0.223	0.5%

[sundb]

Following is the RDB load time test of quicklist ziplist convert to quicklist listpack.
From the results, we can see that the speed of converting quicklist ziplist to quicklist listpackis faster than hash and zset,
which I think is reasonable because loading list key does not need to check duplication.
See #8887 (comment), #9366 (comment).

Config:

list-max-listpack-size: -2
list-compress-depth 0

Test other info:

list entry size: 32 bytes(rand string)

Due to the physical memory limitation, when the list size and quicklist length are 250000/512 and 500000/256, the loading speed drops significantly, we can ignore their loading speed degradation percentage.

list size	quicklist length per list	quicklist ziplist loading time(second)	quicklist listpack loading and convert time(second)	convert cost time(second)	degradation percentage	rdb size
250000	512	9.053	13.468	4.415	48.7%(memory limit reach)	3.7G
250000	256	4.752	6.850	2.098	44.1%	1.9G
250000	128	2.555	3.722	1.167	45.6%	946M
250000	64	1.385	1.999	0.614	44.3%	486M
500000	256	9.214	13.847	4.633	50.2%(memory limit reach)	3.7G
500000	128	4.990	7.170	2.18	43.6%	1.9G
500000	64	2.868	3.961	1.093	38.1%	971M
500000	32	1.703	2.365	0.662	38.8%	505M

[oranagra]

@redis/core-team this is the final step in getting rid of ziplists, it's basically more of the same of what we did for hashes and listpacks.
please take a quick look at the top comment, and also at the benchmarks at the bottom, specifically this one:

[yossigo]

Approving the change, did not code review.

[madolson]

Approve at a high level.

[soloestoy]

Approve the change, did not code review line by line.

[zuiderkwast]

Good job @sundb!

If this is the last usage of ziplist, can we now remove almost all of ziplist.c? It seems that only ziplistGet is used in the conversion.

[sundb]

@zuiderkwast Only ziplsitGet and ziplistValidateIntegrity are still being used.
But internally they depend on other methods, and removing ziplist.c completely means we need to move the rest of the code elsewhere, which doesn't feel good.
Similarly with zipmap.c, which has been left in place.

[zuiderkwast]

OK. I was thinking about deleting the dead code (ziplistInsert, etc. which is at least half of ziplist.c). I suppose instead we will delete ziplist.c in Redis 8 or 9(?) when we no longer support conversion from ziplist.

[sundb]

@zuiderkwast It could be a long, long time, and I hate to see them at lcov with such low coverage.