List types memory tracking is inaccurate

[chakaz]

Create a list with 1,000,000 entries, consuming 1,000 bytes each:

debug populate 1 l 1000 RAND TYPE list ELEMENTS 100000

Then inspect it:

127.0.0.1:6379> memory usage l:0
(integer) 21000048

(this means each element in the list takes 21 bytes)

This is also shown in:

127.0.0.1:6379> info memory
[...]
type_used_memory_list:21000048
[...]

[kostasrim]

Same for strings, this is a duplicate. One sec

[kostasrim]

duplicate of #3723

[romange]

I think this bug is better defined than #3723 because it clearly shows how to reproduce it. In fact, even an external contributor can try fixing it.

[kostasrim]

because it clearly shows how to reproduce it.

It reproduces in the test I added as well 😄 Sure let's keep it though

[romange]

It could be nice if we could account for memory usage for lists in O(1) time.
I suggest introducing a __thread ssize_t cur_list_sz variable in quick_list.c. Before any operation on the list we will copy RobjWrapper::sz_ into it, then do the operation and quicklist code will update it with the size changes.
Once the operation completes we will copy cur_list_sz back into RobjWrapper::sz_ that will hold the total memory size of quicklist->entry data

[romange]

Seems that quicklistNodeUpdateSz and __quicklistCompress are places where we should update cur_list_sz

[andydunstall]

Discussed with roman and agreed this will require bigger changes than expected, so unassigning myself

[chakaz]

@andydunstall / @romange, can you please elaborate on what you've learned that make this bigger than originally expected? Is it not enough to add accounting in quick_list.c? What else is needed?

[romange]

It is enough but I did not feel comfortable to do intrusive changes to quicklist.cc
There are also other reasons why I preferred to move the code ownership under Dragonfly and then develop the code according to our needs.

[romange]

@adiholden I am assigning to myself since I am doing changes in that area.