|From:||"Constantin S(dot) Pan" <kvapen(at)gmail(dot)com>|
|To:||Dmitry Ivanov <d(dot)ivanov(at)postgrespro(dot)ru>|
|Subject:||Re: [WIP] speeding up GIN build with parallel workers|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
On Fri, 18 Mar 2016 20:40:16 +0300
Dmitry Ivanov <d(dot)ivanov(at)postgrespro(dot)ru> wrote:
> Hi Constantin,
> I did a quick review of your patch, and here are my comments:
> - This patch applies cleanly to the current HEAD (61d2ebdbf91).
> - Code compiles without warnings.
> - Currently there's no documentation regarding parallel gin build
> feature and provided GUC variables.
> - Built indexes work seem to work normally.
> I've made a few runs on my laptop (i7-4710HQ, default
> postgresql.conf), here are the results:
> workers avg. time (s)
> 0 412
> 4 133
> 8 81
> Looks like 8 workers & a backend do the job 5x times faster than a
> sole backend, which is good!
> Code style
> There are some things that you've probably overlooked, such as:
> > task->heap_oid = heap->rd_id;
> > task->index_oid = index->rd_id;
> You could replace direct access to 'rd_id' field with the
> RelationGetRelid macro.
> > static void ginDumpEntry(GinState *ginstate,
> > volatile
> > WorkerResult *r
> Parameter 'r' is unused, you could remove it.
> Some of the functions and pieces of code that you've added do not
> comply to the formatting conventions, e. g.
> > static int claimSomeBlocks(...
> > static GinEntryStack *pushEntry(
> >> // The message consists of 2 or 3 parts. iovec allows us to send
> >> them as
> Keep up the good work!
Thank you for the review. I am working on a new version, that will fix
the code style and also involve backend into the process as one of the
|Next Message||Tomas Vondra||2016-03-22 09:13:18||Re: checkpointer continuous flushing|
|Previous Message||Mithun Cy||2016-03-22 08:42:54||Re: POC: Cache data in GetSnapshotData()|