uros-b commented on code in PR #57587:
URL: https://github.com/apache/spark/pull/57587#discussion_r3667498889


##########
python/pyspark/testing/utils.py:
##########
@@ -294,9 +297,58 @@ def __exit__(self, exc_type, exc_val, exc_tb):
 class PySparkBaseTestCase(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
+        if have_grimp and (path := os.environ.get("PYSPARK_CHANGED_FILES")):
+            # PYSPARK_CHANGED_FILES should only be used when ONLY pyspark 
files are changed.
+            # If other files (JVM for example) are changed, do NOT set this.
+            cls.skip_if_changed_files_irrelevant(path)
+
         if os.environ.get("PYSPARK_TEST_TIMEOUT"):
             faulthandler.register(signal.SIGTERM, file=sys.__stderr__, 
all_threads=True)
 
+    @classmethod
+    def skip_if_changed_files_irrelevant(cls, path: str) -> None:
+        module = cls.__module__
+        if module == "__main__":
+            mod = sys.modules["__main__"]
+            if mod.__spec__ and mod.__spec__.name:
+                module = mod.__spec__.name
+            else:
+                return
+
+        if not cls._is_module_relevant_to_changed_files(module, path):
+            raise unittest.SkipTest("Skipping test because changed files are 
irrelevant")
+
+    @staticmethod
+    @functools.cache
+    def _is_module_relevant_to_changed_files(module: str, path: str) -> bool:
+        import grimp
+
+        with open(path, "r") as f:
+            changed_files = f.read().strip().splitlines()
+
+        if not all(f.startswith("python/pyspark/") and f.endswith(".py") for f 
in changed_files):
+            # We have a wrong list of files, just run the test.
+            return True
+
+        changed_modules = [
+            f.removeprefix("python/").rsplit(".", 1)[0].replace(os.path.sep, 
".")
+            for f in changed_files
+        ]
+
+        graph = grimp.build_graph("pyspark")

Review Comment:
   This is called inside _is_module_relevant_to_changed_files, which is cached 
by (module, path). Within a single test process that runs multiple test classes 
(each with a distinct module but the same path), the expensive graph build 
fires once per class rather than once per process. A test module with N test 
classes causes N redundant full-package graph builds. Separating graph 
construction into its own @functools.cache-d helper keyed only on path (since 
the package is always "pyspark") would cap overhead at one build per process.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to