Skip to content

[SPARK-56067][PYTHON] Lazy import psutil to improve import speed#54897

Open
gaogaotiantian wants to merge 1 commit intoapache:masterfrom
gaogaotiantian:lazy-import-psutil
Open

[SPARK-56067][PYTHON] Lazy import psutil to improve import speed#54897
gaogaotiantian wants to merge 1 commit intoapache:masterfrom
gaogaotiantian:lazy-import-psutil

Conversation

@gaogaotiantian
Copy link
Contributor

What changes were proposed in this pull request?

Instead of import psutil at module-level, we do it in the function to avoid always triggering the import when we import pyspark.

Why are the changes needed?

psutil itself is relatively fast to import, but it's also unnecessary. More importantly, it's the only outlier after numpy and memory_profiler. After getting rid of it, we can write a test to check if we import 3rd party library when we do import pyspark` to prevent similar issues in the future.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

test_shuffle passed locally, the rest is on CI.

Was this patch authored or co-authored using generative AI tooling?

No.

def get_used_memory():
"""Return the used memory in MiB"""
try:
import psutil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this will result in the try...except block getting executed multiple times (the import runs each time the function is called).

We could use a strategy that lazily imports, but caches the import in a global variable instead of having Python's import system cache it.

_psutil = None
_psutil_checked = False

def get_used_memory():
    global _psutil, _psutil_checked
    if not _psutil_checked:
        try:
            import psutil
            _psutil = psutil
        except ImportError:
            pass
        _psutil_checked = True
    if _psutil is not None:
        # psutil path
    else:
        # fallback path

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import cache check is basically a dict check in sys.modules, which is super fast compared to the rest of the function. I don't think it's worth it to make the code more complicated. Getting memory usage is expensive (involves IO normally) so optimizing the rest of tis function does not give us much. I prefer to keep code simple unless we have proof that the performance difference is observable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants