pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: WIP: Fast GiST index build
Date: Wed, 10 Aug 2011 22:45:18 +0300
Message-ID: <4E42DFCE.3050904@enterprisedb.com> (raw )
In-Reply-To: <CAPpHfdvi5WZKdRD+s60R+pdm8dLczRQ5_RYt4L8OA_bhorPjpg@mail.gmail.com >
References: <BANLkTikgOQ1+r=A9b08szNTeEWsk-cNMMg@mail.gmail.com >
<CAPpHfdtVJJyinb1TuFF3gw6-YLre2O9svUZgZGAWzogXswP6Bg@mail.gmail.com >
<CAPpHfdvoOh78ycX3eRePTS29635pHCSTfLdFDzHZhxTKsggCuQ@mail.gmail.com >
<4E2DBB60.80604@enterprisedb.com >
<4E2EF83A.8000304@enterprisedb.com >
<CAPpHfdso-oOo4+3r9yBn=PATz3BC4z4cxK9Ew8tFRT1MkVVoiA@mail.gmail.com >
<4E31D03D.70206@enterprisedb.com >
<CAPpHfdtmgwOjNr7J15eCe6sXrtmq_c_52oNh-=D934SKekoGxg@mail.gmail.com >
<CAPpHfdv=HQDmUGTARKjH+AiHMDpXa+8+WwuqjUSfZnVdC-LJbQ@mail.gmail.com >
<CAPpHfduAhSNo8LHMPj_pqQ_2W6ZDPzdQBPzHbnpqaKz6kf4nrw@mail.gmail.com >
<4E3977D8.1020506@enterprisedb.com >
<CAPpHfducs8Ussw+t8iP3WS3ZTuX2J-77FOY=jAZUg-K6AuOC0g@mail.gmail.com >
<CAPpHfdusd_b_dtvNY2uuJict6=VEUTGAS8ELXGck=o=kOdMnOw@mail.gmail.com >
<4E3FAB1D.1060105@enterprisedb.com >
<CAPpHfdsovwbgs30TojJc_zDsc+0ghapm+-AALfP58UaHiYjr2A@mail.gmail.com >
<CAPpHfdvi5WZKdRD+s60R+pdm8dLczRQ5_RYt4L8OA_bhorPjpg@mail.gmail.com >
On 10.08.2011 13:19, Alexander Korotkov wrote:
> Hi!
>
> Here is last verion of the patch.
> List of changes:
> 1) Neighbor relocation and prefetch were removed. They will be supplied as
> separate patches.
unloadNodeBuffers() is now dead code.
> 2) Final emptying now using standart lists of all buffers by levels.
> 3) Automatic switching again use simple comparison of index size and
> effective_cache_size.
LEAF_PAGES_STATS_* are unused now. Should avoid calling smgrnblocks() on
every tuple, the overhead of that could add up.
> 4) Some renames. In particular GISTLoadedPartItem
> to GISTBufferingInsertStack.
> 5) Some comments were corrected and some were added.
> 6) pgindent
> 7) rebased with head
>
> Readme update and user documentation coming soon.
I wonder, how hard would it be to merge gistBufferingBuildPlaceToPage()
with the gistplacetopage() function used in the main codepath? There's
very little difference between them, and it would be nice to maintain
just one function. At the very least I think there should be a comment
in both along the lines of "NOTE: if you change this function, make sure
you update XXXX (the other function) as well!"
In gistbuild(), in the final emptying stage, there's this special-case
handling for the root block before looping through the buffers in the
buffersOnLevels lists:
> for (;;)
> {
> nodeBuffer = getNodeBuffer(gfbb, &buildstate.giststate, GIST_ROOT_BLKNO,
> InvalidOffsetNumber, NULL, false);
> if (!nodeBuffer || nodeBuffer->blocksCount <= 0)
> break;
> MemoryContextSwitchTo(gfbb->context);
> gfbb->bufferEmptyingStack = lcons(nodeBuffer, gfbb->bufferEmptyingStack);
> MemoryContextSwitchTo(buildstate.tmpCtx);
> processEmptyingStack(&buildstate.giststate, &insertstate);
> }
What's the purpose of that? Wouldn't the loop through buffersOnLevels
lists take care of the root node too?
The calculations in initBuffering() desperately need comments. As does
the rest of the code too, but the heuristics in that function are
particularly hard to understand without some explanation.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
view thread (114+ messages) latest in thread
Message-ID: <4E42DFCE.3050904@enterprisedb.com>
Permalink: ../4E42DFCE.3050904@enterprisedb.com/
Also on: postgresql.org/message-id/4E42DFCE.3050904@enterprisedb.com
copy link · copy postgr.es
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: heikki.linnakangas@enterprisedb.com, aekorotkov@gmail.com
Subject: Re: WIP: Fast GiST index build
In-Reply-To: <4E42DFCE.3050904@enterprisedb.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox