Re: pgsql: Add putenv support for msvcrt from Visual Studio 2013

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Christian Ullrich <chris(at)chrullrich(dot)net>
Cc: Noah Misch <noah(at)leadboat(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Magnus Hagander <magnus(at)hagander(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pgsql: Add putenv support for msvcrt from Visual Studio 2013
Date: 2016-11-30 07:56:41
Message-ID: CAB7nPqStLLt9k2SQj_gVhRfbCYUpAUz5OFYu2yeVa_x3na2E6w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On Tue, Nov 29, 2016 at 08:45:13PM +0100, Christian Ullrich wrote:
> * Michael Paquier wrote:
>
> > On Wed, Nov 16, 2016 at 12:45 PM, Christian Ullrich
> > <chris(at)chrullrich(dot)net> wrote:
> >> I also did a debug build with 1+2+4 that came in at 84 μs/iteration.
> >
> > Moved to next CF. Christian, perhaps this patch should have an extra
> > round of reviews?
>
> It is significantly different from the last version, so yes, of course.

Patches 0001 (Cosmetic fixes), 0002 (Remove unnecessary CloseHandle)
and 0003 (support for debug CRTs) look in good shape to me. 0004 (Fix
load race) is 0002 from the previous set, and this change makes sense
in itself.

With 0005 I am seeing a compilation failure: you need to have the
declarations in the _MSC_VER block at the beginning of the routine. It
would be nice to mention in a code comment that this what Noah has
mentioned upthread: if a CRT loads while pgwin32_putenv() is
executing, the newly-loaded CRT will always have the latest value.

Based on that 0006 will need a rebase, nothing huge though.

Removing the caching per the measurements upthread is causing a 1us
regression compared to the full set. Let's do things simple then! This
smells like noise.
--
Michael

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Heikki Linnakangas 2016-11-30 11:04:48 pgsql: Remove dead stuff from pgcrypto.
Previous Message Tom Lane 2016-11-30 00:32:43 pgsql: Fix bogus handling of JOIN_UNIQUE_OUTER/INNER cases for parallel

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2016-11-30 08:01:02 Minor correction in alter_table.sgml
Previous Message Amit Langote 2016-11-30 07:51:26 Minor correction in alter_table.sgml