[Runtime] Unify debug variable parsing from the environment and avoid getenv when possible. #32137

mikeash · 2020-06-02T15:28:55Z

There are a few environment variables used to enable debugging options in the
runtime, and we'll likely add more over time. These are implemented with
scattered getenv() calls at the point of use. This is inefficient, as most/all
OSes have to do a linear scan of the environment for each call. It's also not
discoverable, since the only way to find these variables is to inspect the
source.

This commit places all of these variables in a central location.
stdlib/public/runtime/EnvironmentVariables.def defines all of the debug
variables including their name, type, default value, and a help string. On OSes
which make an environ array available, the entire array is scanned in a single
pass the first time any debug variable is requested. By quickly rejecting
variables that do not start with SWIFT_, we optimize for the common case where
no debug variables are set. We also have a fallback to repeated getenv() calls
when a full scan is not possible.

Setting SWIFT_HELP=YES will print out all available debug variables along with
a brief description of what they do.

compnerd

This definitely is going to improve working on the runtime! Thanks for doing this.

include/swift/Runtime/EnvironmentVariables.h

stdlib/public/runtime/EnvironmentVariables.cpp

compnerd · 2020-06-02T15:49:43Z

stdlib/public/runtime/EnvironmentVariables.cpp

+
+// Define backing variables.
+#define VARIABLE(name, type, defaultValue, help) \
+  type Environment::name ## _variable;


Should this not be static type ...?

No, this and initializeToken get exposed to other files through the inline getter functions in the header, so they need to be non-static.

compnerd · 2020-06-02T15:51:13Z

stdlib/public/runtime/EnvironmentVariables.cpp

+}
+
+// Initialization code.
+OnceToken_t Environment::initializeToken;


Should this not be static?

stdlib/public/runtime/EnvironmentVariables.cpp

test/Runtime/environment_variables.swift

mikeash · 2020-06-03T16:03:56Z

@compnerd Thanks for the feedback! I just pushed changes to address the comments I didn't reply to. Let me know if you see anything else.

mikeash · 2020-06-03T16:17:57Z

@swift-ci please test

swift-ci · 2020-06-03T16:58:55Z

Build failed
Swift Test Linux Platform
Git Sha - a8edacaee4555bc82d4f07a6699194ac173ea223

swift-ci · 2020-06-03T17:32:58Z

Build failed
Swift Test OS X Platform
Git Sha - a8edacaee4555bc82d4f07a6699194ac173ea223

mikeash · 2020-06-03T17:37:57Z

@swift-ci please test

swift-ci · 2020-06-03T17:40:42Z

Build failed
Swift Test Linux Platform
Git Sha - a8edacaee4555bc82d4f07a6699194ac173ea223

swift-ci · 2020-06-03T17:40:51Z

Build failed
Swift Test OS X Platform
Git Sha - a8edacaee4555bc82d4f07a6699194ac173ea223

mikeash · 2020-06-03T20:46:30Z

@swift-ci please test

swift-ci · 2020-06-03T20:49:28Z

Build failed
Swift Test Linux Platform
Git Sha - fbcb335703f06dd763ec18628ad170ba79f00e8a

swift-ci · 2020-06-03T21:37:05Z

Build failed
Swift Test OS X Platform
Git Sha - fbcb335703f06dd763ec18628ad170ba79f00e8a

gribozavr · 2020-06-04T09:14:17Z

include/swift/Runtime/EnvironmentVariables.h

@@ -0,0 +1,41 @@
+//===--- CompatibiltyOverride.h - Debug variables. --------------*- C++ -*-===//


Suggested change

//===--- CompatibiltyOverride.h - Debug variables. --------------*- C++ -*-===//

//===--- EnvironmentVariables.h - Debug variables. --------------*- C++ -*-===//

D'oh, good catch. Thank you!

gribozavr · 2020-06-04T09:14:58Z

stdlib/public/runtime/EnvironmentVariables.cpp

@@ -0,0 +1,182 @@
+//===--- CompatibiltyOverride.h - Debug variables. --------------*- C++ -*-===//


Suggested change

//===--- CompatibiltyOverride.h - Debug variables. --------------*- C++ -*-===//

//===--- EnvironmentVariables.cpp - Debug variables. ------------*- C++ -*-===//

… getenv when possible. There are a few environment variables used to enable debugging options in the runtime, and we'll likely add more over time. These are implemented with scattered getenv() calls at the point of use. This is inefficient, as most/all OSes have to do a linear scan of the environment for each call. It's also not discoverable, since the only way to find these variables is to inspect the source. This commit places all of these variables in a central location. stdlib/public/runtime/EnvironmentVariables.def defines all of the debug variables including their name, type, default value, and a help string. On OSes which make an `environ` array available, the entire array is scanned in a single pass the first time any debug variable is requested. By quickly rejecting variables that do not start with `SWIFT_`, we optimize for the common case where no debug variables are set. We also have a fallback to repeated `getenv()` calls when a full scan is not possible. Setting `SWIFT_HELP=YES` will print out all available debug variables along with a brief description of what they do.

mikeash · 2020-06-04T14:00:32Z

@swift-ci please test

swift-ci · 2020-06-04T14:02:48Z

Build failed
Swift Test Linux Platform
Git Sha - 5ea8bc36eff0766b3e06fe5269ee1c2ed0918bfd

swift-ci · 2020-06-04T14:04:00Z

Build failed
Swift Test OS X Platform
Git Sha - 5ea8bc36eff0766b3e06fe5269ee1c2ed0918bfd

mikeash · 2020-06-04T17:17:54Z

@swift-ci please test

tbkka

Nice! I like how you've organized this. Should be pretty easy to build on in the future.

tbkka · 2020-06-04T17:35:38Z

stdlib/public/runtime/Metadata.cpp

@@ -5655,10 +5656,8 @@ void swift::verifyMangledNameRoundtrip(const Metadata *metadata) {
  // variable lets us easily turn on verification to find and fix these
  // bugs. Remove this and leave it permanently on once everything works
  // with it enabled.
-  bool verificationEnabled =


Remove the last sentence of the comment above, since we no longer plan to remove this verification code.

Good idea. I'll do that in a followup, since I just got the thing through PR testing and I'd rather not continue that particular struggle....

In swiftlang#32137 direct environment variable parsing was introduced, the availability of which is branched by a preprocessor symbol (`ENVIRON`) if the environment is available directly on the platform, or if getenv must be used. The message expectation in the unit test in the however diverged on the non-`ENVIRON` branch, causing a unit test failure. This PR fixes the expectation, but also, while we're here, OpenBSD supports the `ENVIRON` branch anyway.

mikeash requested review from compnerd and tbkka June 2, 2020 15:28

compnerd reviewed Jun 2, 2020

View reviewed changes

mikeash force-pushed the debug-environment-variables branch from da26553 to a8edaca Compare June 3, 2020 16:01

compnerd approved these changes Jun 3, 2020

View reviewed changes

mikeash force-pushed the debug-environment-variables branch from a8edaca to fbcb335 Compare June 3, 2020 17:37

mikeash force-pushed the debug-environment-variables branch from fbcb335 to 5ea8bc3 Compare June 3, 2020 20:46

gribozavr reviewed Jun 4, 2020

View reviewed changes

mikeash force-pushed the debug-environment-variables branch from 5ea8bc3 to f2fb539 Compare June 4, 2020 14:00

tbkka approved these changes Jun 4, 2020

View reviewed changes

mikeash merged commit b25dab2 into swiftlang:master Jun 4, 2020

3405691582 mentioned this pull request Jun 5, 2020

[stdlib][test] Fix message test expectation. #32199

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Runtime] Unify debug variable parsing from the environment and avoid getenv when possible. #32137

[Runtime] Unify debug variable parsing from the environment and avoid getenv when possible. #32137

mikeash commented Jun 2, 2020 •

edited

Loading

compnerd left a comment

compnerd Jun 2, 2020

mikeash Jun 3, 2020

compnerd Jun 2, 2020

mikeash commented Jun 3, 2020

mikeash commented Jun 3, 2020

swift-ci commented Jun 3, 2020

swift-ci commented Jun 3, 2020

mikeash commented Jun 3, 2020

swift-ci commented Jun 3, 2020

swift-ci commented Jun 3, 2020

mikeash commented Jun 3, 2020

swift-ci commented Jun 3, 2020

swift-ci commented Jun 3, 2020

gribozavr Jun 4, 2020

mikeash Jun 4, 2020

gribozavr Jun 4, 2020

mikeash commented Jun 4, 2020

swift-ci commented Jun 4, 2020

swift-ci commented Jun 4, 2020

mikeash commented Jun 4, 2020

tbkka left a comment

tbkka Jun 4, 2020

mikeash Jun 4, 2020

		@@ -0,0 +1,41 @@
		//===--- CompatibiltyOverride.h - Debug variables. --------------- C++ --===//

	//===--- CompatibiltyOverride.h - Debug variables. --------------- C++ --===//
	//===--- EnvironmentVariables.h - Debug variables. --------------- C++ --===//

		@@ -0,0 +1,182 @@
		//===--- CompatibiltyOverride.h - Debug variables. --------------- C++ --===//

	//===--- CompatibiltyOverride.h - Debug variables. --------------- C++ --===//
	//===--- EnvironmentVariables.cpp - Debug variables. ------------- C++ --===//

[Runtime] Unify debug variable parsing from the environment and avoid getenv when possible. #32137

[Runtime] Unify debug variable parsing from the environment and avoid getenv when possible. #32137

Conversation

mikeash commented Jun 2, 2020 • edited Loading

compnerd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikeash commented Jun 3, 2020

mikeash commented Jun 3, 2020

swift-ci commented Jun 3, 2020

swift-ci commented Jun 3, 2020

mikeash commented Jun 3, 2020

swift-ci commented Jun 3, 2020

swift-ci commented Jun 3, 2020

mikeash commented Jun 3, 2020

swift-ci commented Jun 3, 2020

swift-ci commented Jun 3, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikeash commented Jun 4, 2020

swift-ci commented Jun 4, 2020

swift-ci commented Jun 4, 2020

mikeash commented Jun 4, 2020

tbkka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikeash commented Jun 2, 2020 •

edited

Loading