Skip to content

chore: perf of flamepy. - #412

Merged
k82cn merged 4 commits into
xflops:mainfrom
k82cn:flm_perf
Apr 24, 2026
Merged

chore: perf of flamepy.#412
k82cn merged 4 commits into
xflops:mainfrom
k82cn:flm_perf

Conversation

@k82cn

@k82cn k82cn commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces several significant changes, including a transition from second-level to millisecond-level precision for timestamps across the RPC and storage layers, and the addition of comprehensive performance benchmarks for both Flame and Ray. It also refactors the Python SDK installation process to use a prefix-based approach and implements hash-based caching for package installations in the runner to improve startup performance. Key feedback points include the need to prepend rather than overwrite PYTHONPATH and LD_LIBRARY_PATH in the host shim to avoid conflicts with user-defined environments, cleaning up a redundant debug condition in the benchmark script, and reconsidering the use of the --no-deps flag during pip installation to ensure user package dependencies are correctly resolved.

Comment thread executor_manager/src/shims/host_shim.rs Outdated
Comment thread executor_manager/src/shims/host_shim.rs Outdated
Comment on lines +292 to +293
envs.entry("LD_LIBRARY_PATH".to_string())
.or_insert(ld_paths.join(":"));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Similar to PYTHONPATH, LD_LIBRARY_PATH should be prepended rather than only inserted if missing. If a user provides their own LD_LIBRARY_PATH, the native extensions required by packages in the shim's site-packages might fail to load because their containing directories aren't in the search path.

                    let ld_paths_str = ld_paths.join(":");
                    envs.entry("LD_LIBRARY_PATH".to_string())
                        .and_modify(|e| *e = format!("{}:{}", ld_paths_str, e))
                        .or_insert(ld_paths_str);

Comment thread perf/flamepy/main.py Outdated
Comment thread sdk/python/src/flamepy/runner/runpy.py Outdated
@codecov

codecov Bot commented Apr 21, 2026

Copy link
Copy Markdown

@k82cn
k82cn force-pushed the flm_perf branch 10 times, most recently from 20c411c to 660a42f Compare April 23, 2026 08:35
k82cn added 3 commits April 24, 2026 08:13
Signed-off-by: Klaus Ma <klausm@nvidia.com>
Signed-off-by: Klaus Ma <klausm@nvidia.com>
Signed-off-by: Klaus Ma <klausm@nvidia.com>
@k82cn
k82cn merged commit 9020f15 into xflops:main Apr 24, 2026
6 of 7 checks passed
@k82cn
k82cn deleted the flm_perf branch April 24, 2026 09:11
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.

1 participant