Re: [PR] [SPARK-60340][CORE] Avoid System.getenv on every JavaUtils.isTesting call [spark]

["mihailoale-db (via GitHub)" <gi...@apache.org>]

mihailoale-db commented on code in PR #59593:
URL: https://github.com/apache/spark/pull/59593#discussion_r4314090587


##########
common/utils-java/src/main/java/org/apache/spark/network/util/JavaUtils.java:
##########
@@ -696,11 +696,16 @@ public static String toString(InputStream in) throws IOException {
     return new String(in.readAllBytes(), StandardCharsets.UTF_8);
   }
 
+  // The process environment is immutable, and System.getenv(String) allocates on every lookup,
+  // which shows up on hot paths such as Catalyst's per-transform isTesting check. The
+  // spark.testing system property is still read on each call since tests set and clear it.
+  private static final boolean SPARK_TESTING_ENV_SET = System.getenv("SPARK_TESTING") != null;

Review Comment:
   Could we have a simple test checking that `SPARK_TESTING_ENV_SET` indeed doesn't change between runs?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org