[tarantool-patches] Re: [PATCH v3 1/7] memtx: introduce universal iterator_pool

  • From: Vladimir Davydov <vdavydov.dev@xxxxxxxxx>
  • To: Konstantin Osipov <kostja.osipov@xxxxxxxxx>
  • Date: Mon, 25 Feb 2019 20:15:31 +0300

On Mon, Feb 25, 2019 at 07:46:58PM +0300, Konstantin Osipov wrote:

* Vladimir Davydov <vdavydov.dev@xxxxxxxxx> [19/02/24 21:24]:
On Sun, Feb 24, 2019 at 08:15:04PM +0300, Konstantin Osipov wrote:
* Vladimir Davydov <vdavydov.dev@xxxxxxxxx> [19/02/24 10:01]:
On Fri, Feb 22, 2019 at 09:37:25PM +0300, Konstantin Osipov wrote:
* Kirill Shcherbatov <kshcherbatov@xxxxxxxxxxxxx> [19/02/22 19:29]:
Memtx uses separate mempools for iterators of different types.
Due to the fact that there will be more iterators of different
sizes in a series of upcoming changes, let's always allocate the
iterator of the largest size.

If rtree iterator is the one which is largest, let's use a
separate pool for it. 

In general mempools are rather cheap. Each mempool takes a slab
for ~100 objects and uses no slabs if there are no objects (e.g.
if rtree index is not used, there is no mempool memory for it).

But I'd rather prefer to use the same mempool for all kinds of iterator
objects to simplify the code. Take a look at how those mempools are
initialized on demand. IMO it looks ugly. Do we really want to save
those 500 of bytes that much to put up with that complexity?

I don't know much, but a typical SAP R3 which I working on making 
work well with one open source database back in 2003 had ~100k
open client cursors. This could easily entail hundreds of
thousands of server side iterators.

Regarding the bps tree performance issue. I see nothing wrong about it.
We've found an issue and we'll surely fix it. There was no point to
think about such a minor optimization until we actually faced the
problem. My point is we should strive to write simple and reliable code
first, and optimize it only if there's a demand, otherwise we risk
turning the code into unmaintainable mess for no good reason.

I agree with your point, I just disagree that this optimization is
not practical.

Okay, pushed the patch that keeps rtree_iterator_pool.

Other related posts: