From 099fa37bb528b561d472c92ed5b5c244fce38229 Mon Sep 17 00:00:00 2001 From: Andrei Alexandrescu Date: Tue, 23 May 2017 15:40:05 -0400 Subject: [PATCH 1/9] Get rid of static this for initializing std{in,out,err} --- index.d | 1 - posix.mak | 1 - std/stdio.d | 111 +++++++++++++++++++++++------------------------- std/stdiobase.d | 24 ----------- win32.mak | 2 - win64.mak | 1 - 6 files changed, 53 insertions(+), 87 deletions(-) delete mode 100644 std/stdiobase.d diff --git a/index.d b/index.d index b7f5341ebd5..905ebf3cbbf 100644 --- a/index.d +++ b/index.d @@ -470,7 +470,6 @@ $(COMMENT $(LINK2 std_regex_internal_parser.html, std.regex.internal.parser)$(BR) $(LINK2 std_regex_internal_tests.html, std.regex.internal.tests)$(BR) $(LINK2 std_regex_internal_thompson.html, std.regex.internal.thompson)$(BR) - $(LINK2 std_stdiobase.html, std.stdiobase)$(BR) ) $(TD Internal modules. diff --git a/posix.mak b/posix.mak index 2080712aa9c..bce3e08f29b 100644 --- a/posix.mak +++ b/posix.mak @@ -227,7 +227,6 @@ EXTRA_MODULES_INTERNAL := $(addprefix std/, \ processinit scopebuffer test/dummyrange \ $(addprefix unicode_, comp decomp grapheme norm tables) \ ) \ - stdiobase \ ) EXTRA_MODULES += $(EXTRA_DOCUMENTABLES) $(EXTRA_MODULES_INTERNAL) diff --git a/std/stdio.d b/std/stdio.d index 9da7656a4f9..dc723f40734 100644 --- a/std/stdio.d +++ b/std/stdio.d @@ -19,7 +19,6 @@ import std.algorithm.mutation; // copy import std.meta; // allSatisfy import std.range.primitives; // ElementEncodingType, empty, front, // isBidirectionalRange, isInputRange, put -import std.stdiobase; import std.traits; // isSomeChar, isSomeString, Unqual, isPointer import std.typecons; // Flag @@ -4522,70 +4521,66 @@ Initialize with a message and an error code. } } -extern(C) void std_stdio_static_this() +// Undocummented but public because std* handles are aliasing it +ref File makeGlobal(alias handle)() { - static import core.stdc.stdio; - //Bind stdin, stdout, stderr - __gshared File.Impl stdinImpl; - stdinImpl.handle = core.stdc.stdio.stdin; - .stdin._p = &stdinImpl; - // stdout - __gshared File.Impl stdoutImpl; - stdoutImpl.handle = core.stdc.stdio.stdout; - .stdout._p = &stdoutImpl; - // stderr - __gshared File.Impl stderrImpl; - stderrImpl.handle = core.stdc.stdio.stderr; - .stderr._p = &stderrImpl; + static __gshared File.Impl impl; + static __gshared File result; + static shared bool lilyWasHere; + import std.concurrency : initOnce; + initOnce!lilyWasHere({ + impl.handle = handle; + result._p = &impl; + return true; + }()); + return result; } -//--------- -__gshared +/** The standard input stream. + Bugs: + Due to $(LINK2 https://issues.dlang.org/show_bug.cgi?id=15768, bug 15768), + it is thread un-safe to reassign `stdin` to a different `File` instance + than the default. +*/ +alias stdin = makeGlobal!(core.stdc.stdio.stdin); + +/// +@safe unittest { - /** The standard input stream. - Bugs: - Due to $(LINK2 https://issues.dlang.org/show_bug.cgi?id=15768, bug 15768), - it is thread un-safe to reassign `stdin` to a different `File` instance - than the default. - */ - File stdin; - /// - @safe unittest - { - // Read stdin, sort lines, write to stdout - import std.array : array; - import std.algorithm.sorting : sort; - import std.algorithm.mutation : copy; - import std.typecons : Yes; - - void main() { - stdin // read from stdin - .byLineCopy(Yes.keepTerminator) // copying each line - .array() // convert to array of lines - .sort() // sort the lines - .copy( // copy output of .sort to an OutputRange - stdout.lockingTextWriter()); // the OutputRange - } + // Read stdin, sort lines, write to stdout + import std.array : array; + import std.algorithm.sorting : sort; + import std.algorithm.mutation : copy; + import std.typecons : Yes; + + void main() { + stdin // read from stdin + .byLineCopy(Yes.keepTerminator) // copying each line + .array() // convert to array of lines + .sort() // sort the lines + .copy( // copy output of .sort to an OutputRange + stdout.lockingTextWriter()); // the OutputRange } - - /** - The standard output stream. - Bugs: - Due to $(LINK2 https://issues.dlang.org/show_bug.cgi?id=15768, bug 15768), - it is thread un-safe to reassign `stdout` to a different `File` instance - than the default. - */ - File stdout; - /** - The standard error stream. - Bugs: - Due to $(LINK2 https://issues.dlang.org/show_bug.cgi?id=15768, bug 15768), - it is thread un-safe to reassign `stderr` to a different `File` instance - than the default. - */ - File stderr; } +/** + The standard output stream. + Bugs: + Due to $(LINK2 https://issues.dlang.org/show_bug.cgi?id=15768, bug 15768), + it is thread un-safe to reassign `stdout` to a different `File` instance + than the default. +*/ +alias stdout = makeGlobal!(core.stdc.stdio.stdout); + +/** + The standard error stream. + Bugs: + Due to $(LINK2 https://issues.dlang.org/show_bug.cgi?id=15768, bug 15768), + it is thread un-safe to reassign `stderr` to a different `File` instance + than the default. +*/ +alias stderr = makeGlobal!(core.stdc.stdio.stderr); + @system unittest { static import std.file; diff --git a/std/stdiobase.d b/std/stdiobase.d deleted file mode 100644 index 4570a4c9c11..00000000000 --- a/std/stdiobase.d +++ /dev/null @@ -1,24 +0,0 @@ -// Written in the D programming language. - -/** - * The only purpose of this module is to do the static construction for - * std.stdio, to eliminate cyclic construction errors. - * - * Copyright: Copyright Andrei Alexandrescu 2008 - 2009. - * License: $(HTTP www.boost.org/LICENSE_1_0.txt, Boost License 1.0). - * Authors: $(HTTP erdani.org, Andrei Alexandrescu) - * Source: $(PHOBOSSRC std/_stdiobase.d) - */ -/* Copyright Andrei Alexandrescu 2008 - 2009. - * Distributed under the Boost Software License, Version 1.0. - * (See accompanying file LICENSE_1_0.txt or copy at - * http://www.boost.org/LICENSE_1_0.txt) - */ -module std.stdiobase; - -extern(C) void std_stdio_static_this(); - -shared static this() -{ - std_stdio_static_this(); -} diff --git a/win32.mak b/win32.mak index 3a9f3b48d37..440bda2385b 100644 --- a/win32.mak +++ b/win32.mak @@ -108,7 +108,6 @@ SRC= \ # The separation is a workaround for bug 4904 (optlink bug 3372). SRC_STD_1= \ std\stdio.d \ - std\stdiobase.d \ std\string.d \ std\format.d \ std\file.d @@ -604,7 +603,6 @@ cov : $(SRC_TO_COMPILE) $(LIB) # cov del *.lst $(DMD) -conf= -cov=83 -unittest -main -run std\stdio.d - $(DMD) -conf= -cov=100 -unittest -main -run std\stdiobase.d $(DMD) -conf= -cov=95 -unittest -main -run std\string.d $(DMD) -conf= -cov=71 -unittest -main -run std\format.d $(DMD) -conf= -cov=83 -unittest -main -run std\file.d diff --git a/win64.mak b/win64.mak index 80bc8467ea0..99826d9c3c0 100644 --- a/win64.mak +++ b/win64.mak @@ -112,7 +112,6 @@ SRC= \ # The separation is a workaround for bug 4904 (optlink bug 3372). SRC_STD_1= \ std\stdio.d \ - std\stdiobase.d \ std\string.d \ std\format.d \ std\file.d From a85409262293a70cee8cdcb5cae0bd4656b5948e Mon Sep 17 00:00:00 2001 From: Andrei Alexandrescu Date: Tue, 23 May 2017 19:21:36 -0400 Subject: [PATCH 2/9] Make it less fun --- std/stdio.d | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/std/stdio.d b/std/stdio.d index dc723f40734..8f193bd2b43 100644 --- a/std/stdio.d +++ b/std/stdio.d @@ -4526,9 +4526,9 @@ ref File makeGlobal(alias handle)() { static __gshared File.Impl impl; static __gshared File result; - static shared bool lilyWasHere; + static shared bool initialized; import std.concurrency : initOnce; - initOnce!lilyWasHere({ + initOnce!initialized({ impl.handle = handle; result._p = &impl; return true; From 38ebccd929888bf0d555936bdf598ff3acccc2db Mon Sep 17 00:00:00 2001 From: Andrei Alexandrescu Date: Wed, 24 May 2017 17:28:01 +0100 Subject: [PATCH 3/9] initOnce is slow, use double checking --- std/stdio.d | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/std/stdio.d b/std/stdio.d index 8f193bd2b43..25063e12d7f 100644 --- a/std/stdio.d +++ b/std/stdio.d @@ -4521,18 +4521,22 @@ Initialize with a message and an error code. } } -// Undocummented but public because std* handles are aliasing it +// Undocummnted but public because the std* handles are aliasing it. ref File makeGlobal(alias handle)() { static __gshared File.Impl impl; static __gshared File result; static shared bool initialized; - import std.concurrency : initOnce; - initOnce!initialized({ - impl.handle = handle; - result._p = &impl; - return true; - }()); + if (!initialized) + { + // Use double-checking because initOnce is inefficient. + import std.concurrency : initOnce; + initOnce!initialized({ + impl.handle = handle; + result._p = &impl; + return true; + }()); + } return result; } From ee318374bd7938e9156e2c29da53785d65958e36 Mon Sep 17 00:00:00 2001 From: Andrei Alexandrescu Date: Thu, 25 May 2017 00:45:35 +0100 Subject: [PATCH 4/9] Alternative implementation that doesn't use initOnce --- std/stdio.d | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/std/stdio.d b/std/stdio.d index 25063e12d7f..132b47aca88 100644 --- a/std/stdio.d +++ b/std/stdio.d @@ -4526,16 +4526,23 @@ ref File makeGlobal(alias handle)() { static __gshared File.Impl impl; static __gshared File result; - static shared bool initialized; + static shared uint initialized; if (!initialized) { - // Use double-checking because initOnce is inefficient. - import std.concurrency : initOnce; - initOnce!initialized({ - impl.handle = handle; - result._p = &impl; - return true; - }()); + for (;;) + { + if (initialized > uint.max / 2) + break; + import core.atomic; + if (atomicOp!"+="(initialized, 1) == 1) + { + impl.handle = handle; + result._p = &impl; + atomicOp!"+="(initialized, uint.max / 2); + break; + } + atomicOp!"-="(initialized, 1); + } } return result; } From 3f583249f8a315515918c92ddc513f464b9d27b5 Mon Sep 17 00:00:00 2001 From: Andrei Alexandrescu Date: Thu, 25 May 2017 01:45:23 +0100 Subject: [PATCH 5/9] Use explicit atomic loading for the double-checked variable --- std/stdio.d | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/std/stdio.d b/std/stdio.d index 132b47aca88..f911f4b6cea 100644 --- a/std/stdio.d +++ b/std/stdio.d @@ -4527,22 +4527,16 @@ ref File makeGlobal(alias handle)() static __gshared File.Impl impl; static __gshared File result; static shared uint initialized; - if (!initialized) - { - for (;;) - { - if (initialized > uint.max / 2) - break; - import core.atomic; - if (atomicOp!"+="(initialized, 1) == 1) - { - impl.handle = handle; - result._p = &impl; - atomicOp!"+="(initialized, uint.max / 2); - break; - } - atomicOp!"-="(initialized, 1); - } + import core.atomic; + if (!atomicLoad(initialized)) + { + // Use double-checking because initOnce is inefficient. + import std.concurrency : initOnce; + initOnce!initialized({ + impl.handle = handle; + result._p = &impl; + return true; + }()); } return result; } From d66e18a5d5ce9d7354b92fb625ef348138f791e9 Mon Sep 17 00:00:00 2001 From: Andrei Alexandrescu Date: Thu, 25 May 2017 01:48:29 +0100 Subject: [PATCH 6/9] Add atomicLoad to the uint-based solution --- std/stdio.d | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/std/stdio.d b/std/stdio.d index f911f4b6cea..d8bc234364b 100644 --- a/std/stdio.d +++ b/std/stdio.d @@ -4530,13 +4530,19 @@ ref File makeGlobal(alias handle)() import core.atomic; if (!atomicLoad(initialized)) { - // Use double-checking because initOnce is inefficient. - import std.concurrency : initOnce; - initOnce!initialized({ - impl.handle = handle; - result._p = &impl; - return true; - }()); + for (;;) + { + if (atomicLoad(initialized) > uint.max / 2) + break; + if (atomicOp!"+="(initialized, 1) == 1) + { + impl.handle = handle; + result._p = &impl; + atomicOp!"+="(initialized, uint.max / 2); + break; + } + atomicOp!"-="(initialized, 1); + } } return result; } From 3fd1bc6fc5bb3ecc2bad0ba56c86afd20a480b88 Mon Sep 17 00:00:00 2001 From: David Nadlinger Date: Sat, 27 May 2017 02:59:45 +0100 Subject: [PATCH 7/9] std.stdio.makeGlobal: Fixup double-checked pattern --- std/stdio.d | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/std/stdio.d b/std/stdio.d index d8bc234364b..29c0b4da0c1 100644 --- a/std/stdio.d +++ b/std/stdio.d @@ -4528,7 +4528,7 @@ ref File makeGlobal(alias handle)() static __gshared File result; static shared uint initialized; import core.atomic; - if (!atomicLoad(initialized)) + if (atomicLoad(initialized) <= uint.max / 2) { for (;;) { From 5a3b007409fffa6cec4787d03eb91b97973abf90 Mon Sep 17 00:00:00 2001 From: David Nadlinger Date: Sat, 27 May 2017 03:11:50 +0100 Subject: [PATCH 8/9] std.stdio.makeGlobal: Add explanatory comment --- std/stdio.d | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/std/stdio.d b/std/stdio.d index 29c0b4da0c1..50e82408dba 100644 --- a/std/stdio.d +++ b/std/stdio.d @@ -4521,27 +4521,32 @@ Initialize with a message and an error code. } } -// Undocummnted but public because the std* handles are aliasing it. +// Undocumented but public because the std* handles are aliasing it. ref File makeGlobal(alias handle)() { static __gshared File.Impl impl; static __gshared File result; - static shared uint initialized; + + // Use an inline spinlock to make sure the initializer is only run once. + // We assume there will be at most uint.max / 2 threads trying to initialize + // `handle` at once and steal the high bit to indicate that the globals have + // been initialized. + static shared uint spinlock; import core.atomic; - if (atomicLoad(initialized) <= uint.max / 2) + if (atomicLoad(spinlock) <= uint.max / 2) { for (;;) { - if (atomicLoad(initialized) > uint.max / 2) + if (atomicLoad(spinlock) > uint.max / 2) break; - if (atomicOp!"+="(initialized, 1) == 1) + if (atomicOp!"+="(spinlock, 1) == 1) { impl.handle = handle; result._p = &impl; - atomicOp!"+="(initialized, uint.max / 2); + atomicOp!"+="(spinlock, uint.max / 2); break; } - atomicOp!"-="(initialized, 1); + atomicOp!"-="(spinlock, 1); } } return result; From 18426550688a7b1e45885a5febda86659427268c Mon Sep 17 00:00:00 2001 From: David Nadlinger Date: Sat, 27 May 2017 03:12:55 +0100 Subject: [PATCH 9/9] std.stdio.makeGlobal: Optimize fast path, MemoryOrder.seq is overkill --- std/stdio.d | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/std/stdio.d b/std/stdio.d index 50e82408dba..cf5eb48cd15 100644 --- a/std/stdio.d +++ b/std/stdio.d @@ -4533,11 +4533,11 @@ ref File makeGlobal(alias handle)() // been initialized. static shared uint spinlock; import core.atomic; - if (atomicLoad(spinlock) <= uint.max / 2) + if (atomicLoad!(MemoryOrder.acq)(spinlock) <= uint.max / 2) { for (;;) { - if (atomicLoad(spinlock) > uint.max / 2) + if (atomicLoad!(MemoryOrder.acq)(spinlock) > uint.max / 2) break; if (atomicOp!"+="(spinlock, 1) == 1) {