Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko <alexander.turenko@tarantool.org>
To: Kirill Yukhin <kyukhin@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [small] Revert "Free all slabs on region reset"
Date: Wed, 29 Jan 2020 11:06:45 +0300	[thread overview]
Message-ID: <4e734e626aba336b27ec85790747c657d29c0338.1580284383.git.alexander.turenko@tarantool.org> (raw)

This reverts commit 67d7ab44ab09df3356929e3692a03321b31f3ebb.

The goal of the reverted commit was to fix flaky fails of tarantool
tests that checks amount of memory used by a fiber:

 | fiber.info()[fiber.self().id()].memory.used

It also attempts to overcome the situation when a fiber holds some
amount of memory, which is not used in any way. The high limit of such
memory is controlled by a threshold in fiber_gc() tarantool's function
(128 KiB at the moment):

 | void
 | fiber_gc(void)
 | {
 |         if (region_used(&fiber()->gc) < 128 * 1024) {
 |                 region_reset(&fiber()->gc);
 |                 return;
 |         }
 |
 |         region_free(&fiber()->gc);
 | }

The reverted commit, however, leads to significant performance
degradation on certain workloads (see #4736). So the revertion fixes the
performance degradation and opens the problem with tests, which is
tracked in #4750.

Related to #12
Related to https://github.com/tarantool/tarantool/issues/4750
Fixes https://github.com/tarantool/tarantool/issues/4736
---

https://github.com/tarantool/small/tree/Totktonada/gh-4736-revert-region-reset

 small/region.h | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/small/region.h b/small/region.h
index d9be176..bea88c6 100644
--- a/small/region.h
+++ b/small/region.h
@@ -156,16 +156,6 @@ region_reserve(struct region *region, size_t size)
 						       slab.next_in_list);
 		if (size <= rslab_unused(slab))
 			return (char *) rslab_data(slab) + slab->used;
-		/* Try to get a slab from the region cache. */
-		slab = rlist_last_entry(&region->slabs.slabs,
-					struct rslab,
-					slab.next_in_list);
-		if (slab->used == 0 && size <= rslab_unused(slab)) {
-			/* Move this slab to the head. */
-			slab_list_del(&region->slabs, &slab->slab, next_in_list);
-			slab_list_add(&region->slabs, &slab->slab, next_in_list);
-			return (char *) rslab_data(slab);
-		}
 	}
 	return region_reserve_slow(region, size);
 }
@@ -222,14 +212,14 @@ region_aligned_alloc(struct region *region, size_t size, size_t alignment)
 
 /**
  * Mark region as empty, but keep the blocks.
- * Do not change the first slab and use previous slabs as a cache to
- * use for future allocations.
  */
 static inline void
 region_reset(struct region *region)
 {
-	struct rslab *slab;
-	rlist_foreach_entry(slab, &region->slabs.slabs, slab.next_in_list) {
+	if (! rlist_empty(&region->slabs.slabs)) {
+		struct rslab *slab = rlist_first_entry(&region->slabs.slabs,
+						       struct rslab,
+						       slab.next_in_list);
 		region->slabs.stats.used -= slab->used;
 		slab->used = 0;
 	}
-- 
2.22.0

             reply	other threads:[~2020-01-29  8:06 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-29  8:06 Alexander Turenko [this message]
2020-01-29  9:59 ` Kirill Yukhin
2020-01-29 21:41 ` Konstantin Osipov
2020-01-30  7:23   ` Alexander Turenko
2020-01-30  8:21     ` Konstantin Osipov
2020-01-30 11:03       ` Alexander Turenko
2020-01-30 12:16         ` Konstantin Osipov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4e734e626aba336b27ec85790747c657d29c0338.1580284383.git.alexander.turenko@tarantool.org \
    --to=alexander.turenko@tarantool.org \
    --cc=kyukhin@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [small] Revert "Free all slabs on region reset"' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox