Skip to content

Fix: device log detection uses mtime instead of filename set diff#738

Open
indigo1973 wants to merge 1 commit into
hw-native-sys:mainfrom
indigo1973:swim_0511
Open

Fix: device log detection uses mtime instead of filename set diff#738
indigo1973 wants to merge 1 commit into
hw-native-sys:mainfrom
indigo1973:swim_0511

Conversation

@indigo1973
Copy link
Copy Markdown
Contributor

@indigo1973 indigo1973 commented May 11, 2026

When a device has been used before (log directory already exists), the
CANN driver creates the current process's .log file during
build_callable, before _snapshot_time runs. The original set-difference
approach captured this file in the snapshot, causing _wait_new_device_log
to spin for a full 15 s timeout. On first use of a device (no existing
log directory), the .log file is created later during _run_and_validate,
so the set-difference happened to work.

Switch to timestamp-based detection: _snapshot_time records the current
time, and _wait_new_device_log looks for any .log file whose mtime is
newer than that timestamp. This correctly detects the file regardless of
when it was created, as long as it is written to during the run.

Copy link
Copy Markdown

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

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 refactors device log tracking to use timestamps instead of set-based comparisons. Reviewers identified an unused parameter in _snapshot_device_logs and suggested optimizing _wait_new_device_log to reduce redundant system calls and handle potential race conditions.

Comment thread simpler_setup/scene_test.py Outdated
Comment thread simpler_setup/scene_test.py Outdated
@indigo1973
Copy link
Copy Markdown
Contributor Author

/gemini review

Copy link
Copy Markdown

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

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 refactors the device log detection logic in simpler_setup/scene_test.py by replacing the file-set snapshot approach with a timestamp-based baseline. The _wait_new_device_log function now identifies new logs by comparing their modification times against this baseline. Feedback suggests using nanosecond-precision timestamps (time.time_ns() and st_mtime_ns) instead of floating-point seconds to ensure more robust file detection and avoid precision issues.

Comment thread simpler_setup/scene_test.py Outdated
Comment thread simpler_setup/scene_test.py Outdated
When a device has been used before (log directory already exists), the
CANN driver creates the current process's .log file during
build_callable, before _snapshot_time runs. The original set-difference
approach captured this file in the snapshot, causing _wait_new_device_log
to spin for a full 15 s timeout. On first use of a device (no existing
log directory), the .log file is created later during _run_and_validate,
so the set-difference happened to work.

Switch to timestamp-based detection: _snapshot_time records the current
time, and _wait_new_device_log looks for any .log file whose mtime is
newer than that timestamp. This correctly detects the file regardless of
when it was created, as long as it is written to during the run.
@indigo1973
Copy link
Copy Markdown
Contributor Author

/gemini review

Copy link
Copy Markdown

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

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 refactors the device log detection mechanism in simpler_setup/scene_test.py by transitioning from a file-set comparison approach to a timestamp-based approach. Specifically, it replaces the _snapshot_device_logs function with _snapshot_time to capture a baseline nanosecond timestamp and updates _wait_new_device_log to identify new logs by comparing their modification times against this baseline. I have no feedback to provide.

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