* Noah Misch wrote: > On Tue, Apr 26, 2016 at 07:39:29PM +0200, Christian Ullrich > wrote:
> > * Christian Ullrich wrote:
> Patch 1 looks good, except that it should be three patches:
>
> - cosmetic parts: change whitespace and s/0/NULL/
> - remove CloseHandle() call
> - probe for debug CRT modules, not just release CRT modules
Attached as 0001, 0002, 0003, in that order.
0004 is what used to be 0002, it disables caching of "DLL not
loaded" results.
> I recommend also moving the SetEnvironmentVariable() call before
> the putenv calls. That way, if a CRT loads while pgwin32_putenv()
> is executing, the newly-loaded CRT will always have the latest
> value.
Agreed, attached as 0005.
0006 was previously known as 0004, removing all caching.
> > Even with patch 0004, there is still a race condition between
> > the main thread and a theoretical additional thread created by
> > some other component that unloads some CRT between
> > GetProcAddress() and the _putenv() call, but that is hardly
> > fixable.
>
> I think you can fix it by abandoning GetModuleHandle() in favor
> of GetModuleHandleEx() + GetProcessAddress() + FreeLibrary().
Attached as 0007.
> > There is another possible fix, ugly as sin, if we really need
> > to keep the whole environment update machinery *and* cannot do
> > the full loop each time. Patch 0003 pins each CRT when we see
> > it for the first time.
This is now 0008.
Patch topology: master --- 1 .. 5 --- 6 --- 7
\
`- 8
> I prefer the simplicity of abandoning the cache (patch 4), if it
> performs decently. Would you compare the performance of patch 1,
> patches 1+2+3, and patches 1+2+4? This should measure the right
> thing (after substituting @libdir@ for your environment):
Tested with release builds; Core i7-6700K (quad/HT; 4000 MHz).
I did three runs each, and they were always within 0.5 % of each
other's run time.
- patch 1 (now 1-3): 24 μs/iteration (24 s for 1e6)
- patch 1+2+3 (now 1-5 + 8): 29 μs/iteration
- patch 1+2+4 (now 1-7): 30 μs/iteration
I also did a debug build with 1+2+4 that came in at 84 μs/iteration.
--
Christian
... now how do I get all the dangling debris out of the repo ...
0008-getmodulehandleex-pin.patch
Description: 0008-getmodulehandleex-pin.patch
0001-whitespace.patch
Description: 0001-whitespace.patch
0002-closehandle.patch
Description: 0002-closehandle.patch
0003-debug-crt.patch
Description: 0003-debug-crt.patch
0004 no-caching-notfound.patch
Description: 0004 no-caching-notfound.patch
0005-reorder-update.patch
Description: 0005-reorder-update.patch
0006-no-caching-at-all.patch
Description: 0006-no-caching-at-all.patch
0007-getmodulehandleex-correctness.patch
Description: 0007-getmodulehandleex-correctness.patch
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
