iobuf arenas: remove arenas that are smaller than 128K - they just take up memory

[mykaul]

Following https://github.com/gluster/glusterfs/commit/491e9ec38bbfedf3caed52535b718efdc2dd6117, which changed the way we use arenas - we now use them only for requests that are larger than 128K, we should remove the smaller areans.

1. We probably need some more arenas between 128K and the next one - 256K.
2. I'm unsure if just 2 arenas of 1MB make sense.
3. There's a significant hit after this change to debug build, due to memory invalidation - a quick hack was to use the smaller arenas, and this won't be feasible after this change...

[mykaul]

Quick fix:

diff --git a/libglusterfs/src/iobuf.c b/libglusterfs/src/iobuf.c
index 445d6cc8c4..06174bfe3e 100644
--- a/libglusterfs/src/iobuf.c
+++ b/libglusterfs/src/iobuf.c
@@ -23,8 +23,10 @@
 /* Make sure this array is sorted based on pagesize */
 static const struct iobuf_init_config gf_iobuf_init_config[] = {
     /* { pagesize, num_pages }, */
-    {128, 1024},     {512, 512},       {2 * 1024, 512}, {8 * 1024, 128},
-    {32 * 1024, 64}, {128 * 1024, 32}, {256 * 1024, 8}, {1 * 1024 * 1024, 2},
+    {128 * 1024, 64},
+    {133, 64},
+    {256 * 1024, 8},
+    {1 * 1024 * 1024, 2},
 };
 
 static int
@@ -560,7 +562,7 @@ iobuf_get2(struct iobuf_pool *iobuf_pool, size_t page_size)
        page size is less than equal to 128KB, the data is available
        on the link https://github.com/gluster/glusterfs/issues/2771
     */
-    if (page_size <= USE_IOBUF_POOL_IF_SIZE_GREATER_THAN) {
+    if (page_size < USE_IOBUF_POOL_IF_SIZE_GREATER_THAN) {
         iobuf = iobuf_get_from_small(page_size);
         if (!iobuf)
             gf_smsg(THIS->name, GF_LOG_WARNING, 0, LG_MSG_IOBUF_NOT_FOUND,

[mohit84]

Quick fix:

diff --git a/libglusterfs/src/iobuf.c b/libglusterfs/src/iobuf.c
index 445d6cc8c4..06174bfe3e 100644
--- a/libglusterfs/src/iobuf.c
+++ b/libglusterfs/src/iobuf.c
@@ -23,8 +23,10 @@
 /* Make sure this array is sorted based on pagesize */
 static const struct iobuf_init_config gf_iobuf_init_config[] = {
     /* { pagesize, num_pages }, */
-    {128, 1024},     {512, 512},       {2 * 1024, 512}, {8 * 1024, 128},
-    {32 * 1024, 64}, {128 * 1024, 32}, {256 * 1024, 8}, {1 * 1024 * 1024, 2},
+    {128 * 1024, 64},
+    {133, 64},
+    {256 * 1024, 8},
+    {1 * 1024 * 1024, 2},
 };
 
 static int
@@ -560,7 +562,7 @@ iobuf_get2(struct iobuf_pool *iobuf_pool, size_t page_size)
        page size is less than equal to 128KB, the data is available
        on the link https://github.com/gluster/glusterfs/issues/2771
     */
-    if (page_size <= USE_IOBUF_POOL_IF_SIZE_GREATER_THAN) {
+    if (page_size < USE_IOBUF_POOL_IF_SIZE_GREATER_THAN) {
         iobuf = iobuf_get_from_small(page_size);
         if (!iobuf)
             gf_smsg(THIS->name, GF_LOG_WARNING, 0, LG_MSG_IOBUF_NOT_FOUND,

Why do we want to ignore standard allocation for 128k ?

[mykaul]

128K happens to be the default for all those that do not use _get2() and therefore is therefore quite frequent allocation request.

• The impact (with debug enabled) due to the memory invalidation is fairly large.
• It is also unclear to me tcmalloc has a real advantage with 128K over statically pre-allocated 128K iobufs (notice that I have pre-allocated 64 of them - and added another 64 of those 133K - which fit 128K + 4K + 140 bytes - also a popular choice here and there! 64 of course is not just a magic number - I'd like to at some point try out to convert those lists to a 64bit bitmap array. I've also seen on my laptop (NVMe) that it's reasonable number)

[mohit84]

128K happens to be the default for all those that do not use _get2() and therefore is therefore quite frequent allocation request.

• The impact (with debug enabled) due to the memory invalidation is fairly large.
• It is also unclear to me tcmalloc has a real advantage with 128K over statically pre-allocated 128K iobufs (notice that I have pre-allocated 64 of them - and added another 64 of those 133K - which fit 128K + 4K + 140 bytes - also a popular choice here and there! 64 of course is not just a magic number - I'd like to at some point try out to convert those lists to a 64bit bitmap array. I've also seen on my laptop (NVMe) that it's reasonable number)

I think we need to test it, afaik current iobuf approach is not perfect we are using single list may be after using bitmap array the performance will not hurt.

[mykaul]

I have the bitmap code somewhere, I can resurrect it once you are done with your outstanding PRs. I'm also trying to think of a clever way to use a bitmap smaller than 64 slots (for the 1MB - or perhaps we don't need that 1MB anyway?), without keeping a counter. I could probably initialize it with fewer slots available or something.

[mohit84]

Ack !!

[stale[bot]]

Thank you for your contributions. Noticed that this issue is not having any activity in last ~6 months! We are marking this issue as stale because it has not had recent activity. It will be closed in 2 weeks if no one responds with a comment here.

[mykaul]

not sure how to unstale it, but I think it's still an easy one I'd like to tackle.

[mohit84]

IIRC earlier we were getting crash in SSL api.

[mykaul]

IIRC earlier we were getting crash in SSL api.

I thought we've fixed it by moving to tcmalloc minimal?

• It can't hurt to remove those iobuf sizes that are <= 128K, since I assume they are not utilized
• I'd like to see if we can remove the standard allocation arena - and just call the regular allocation
• I personally think the only useful arena is 136K - 128K + 4K + some... This is going later. I'll try to split to multiple patches and see where we are at.

[stale[bot]]

Thank you for your contributions. Noticed that this issue is not having any activity in last ~6 months! We are marking this issue as stale because it has not had recent activity. It will be closed in 2 weeks if no one responds with a comment here.

[mykaul]

I still think it's memory completely unused. Unsure why my PR is not getting through - there's a crash in glfs I can't seem to reproduce.

[stale[bot]]

Thank you for your contributions. Noticed that this issue is not having any activity in last ~6 months! We are marking this issue as stale because it has not had recent activity. It will be closed in 2 weeks if no one responds with a comment here.