From: Konstantin Osipov <kostja@tarantool.org> To: Vladimir Davydov <vdavydov.dev@gmail.com> Cc: tarantool-patches@freelists.org Subject: Re: [PATCH 7/8] memtx: rework background garbage collection procedure Date: Tue, 22 May 2018 16:56:34 +0300 [thread overview] Message-ID: <20180522135634.GG11201@atlas> (raw) In-Reply-To: <fca9ed8296d3f49b8ea253df2a4d0891858ab8e7.1526987033.git.vdavydov.dev@gmail.com> * Vladimir Davydov <vdavydov.dev@gmail.com> [18/05/22 15:10]: > +/** > + * Run one iteration of garbage collection. Return true if > + * there still may be objects to collect, false otherwise. > + */ > +static bool > +memtx_engine_run_gc(struct memtx_engine *memtx) > +{ > + if (stailq_empty(&memtx->gc_queue)) > + return false; > + > + struct memtx_gc_task *task = stailq_shift_entry(&memtx->gc_queue, > + struct memtx_gc_task, link); > + if (task->func(task)) { > + /* > + * The task still has objects to collect, > + * put it back. > + */ > + stailq_add_entry(&memtx->gc_queue, task, link); > + } > + return true; > +} Please rework this to avoid adding back tasks to the queue. Peek the task in the queue and pop it if it's complete. Using boolean for task completion status makes the code hard to follow. I would use an out parameter. > memtx_engine_gc_f(va_list va) > { > struct memtx_engine *memtx = va_arg(va, struct memtx_engine *); > while (!fiber_is_cancelled()) { > - if (stailq_empty(&memtx->gc_queue)) { > + if (!memtx_engine_run_gc(memtx)) { This is also counter to our typical use of return values. Otherwise OK to push. -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov
next prev parent reply other threads:[~2018-05-22 13:56 UTC|newest] Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-05-22 11:46 [PATCH 0/8] Follow-up on async memtx index cleanup Vladimir Davydov 2018-05-22 11:46 ` [PATCH 1/8] memtx: init index extent allocator in engine constructor Vladimir Davydov 2018-05-22 13:43 ` Konstantin Osipov 2018-05-22 11:46 ` [PATCH 2/8] memtx: fold memtx_tuple.cc into memtx_engine.c Vladimir Davydov 2018-05-22 13:45 ` Konstantin Osipov 2018-05-22 11:46 ` [PATCH 3/8] memtx: pass engine to memory allocation functions Vladimir Davydov 2018-05-22 13:47 ` Konstantin Osipov 2018-05-22 14:39 ` Vladimir Davydov 2018-05-22 11:46 ` [PATCH 4/8] memtx: move all global variables to engine Vladimir Davydov 2018-05-22 13:48 ` Konstantin Osipov 2018-05-22 11:46 ` [PATCH 5/8] memtx: destroy slab arena on engine shutdown Vladimir Davydov 2018-05-22 13:50 ` Konstantin Osipov 2018-05-22 16:26 ` Vladimir Davydov 2018-05-22 11:46 ` [PATCH 6/8] memtx: embed light hash into memtx_hash_index Vladimir Davydov 2018-05-22 13:51 ` Konstantin Osipov 2018-05-22 11:46 ` [PATCH 7/8] memtx: rework background garbage collection procedure Vladimir Davydov 2018-05-22 13:56 ` Konstantin Osipov [this message] 2018-05-22 14:49 ` Vladimir Davydov 2018-05-22 16:42 ` Konstantin Osipov 2018-05-22 11:46 ` [PATCH 8/8] memtx: run garbage collection on demand Vladimir Davydov 2018-05-22 14:00 ` 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=20180522135634.GG11201@atlas \ --to=kostja@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=vdavydov.dev@gmail.com \ --subject='Re: [PATCH 7/8] memtx: rework background garbage collection procedure' \ /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