Skip to content

Refactor:hook implementation with centralized type definitions and improved enums #115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Refactor:hook implementation with centralized type definitions and improved enums #115

wants to merge 1 commit into from

Conversation

miaobyte
Copy link

This PR improves the codebase architecture by:

  1. Extracting shared hook mechanisms into a centralized hook.h file
  2. Reorganizing the NVML enum structure in libnvml_hook.h with better categorization
  3. Creating consistency between CUDA and NVML hook implementations

Changes

  • Moved common definitions to hook.h:

    • entry_t structure
    • OVERRIDE_ENUM macro
    • FIND_ENTRY macro

  • Reorganized nvml_override_enum_t in libnvml_hook.h:

    • Grouped related functions with clear category comments
    • Followed the same pattern as cuda_override_enum_t for consistency
    • Improved readability with semantic grouping

Benefits

  • Reduced code duplication: Common hook infrastructure now lives in one place
  • Improved maintainability: Changes to hook mechanism only need to be made in one file
  • Better readability: NVML enum is now properly categorized and easier to navigate
  • Consistent style: CUDA and NVML hook implementations now follow the same pattern

Testing

Verified that all existing functionality continues to work as expected.

factor:entry_t
2dec455 
Signed-off-by: peng.li24 <[email protected]>
@hami-robot
Copy link
Contributor

hami-robot bot commented Aug 12, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: miaobyte

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@miaobyte
Copy link
Author

NVML_OVERRIDE_ENUM_t Refactoring Verification

Overview

A comprehensive verification was performed to ensure that no enumeration members were lost or added during the refactoring of the NVML_OVERRIDE_ENUM_t type in libnvml_hook.h. This was critical as this enumeration defines all NVML functions that HAMi intercepts.

Verification Method

  1. Extracted function names from the new version:

    grep "OVERRIDE_ENUM" src/include/libnvml_hook.h | cut -c 21- | sort > ../new.txt

  2. Reset to previous commit to access the original file:

    git reset --hard ee24b8e0715f7f9ff3e03cd2e9212cef62e19959

  3. Extracted function names from the old version:

    grep "NVML_OVERRIDE_ENUM" src/include/libnvml_hook.h | cut -c 21- | sort > ../old.txt

  4. Compared both files to verify no differences:

    diff ../old.txt ../new.txt

Results

The verification confirmed that all NVML functions are preserved in the refactored enum:

  • No functions were added
  • No functions were removed
  • Only the organization and documentation comments were changed

This ensures that the hook mechanism continues to work correctly with all previously supported NVML functions.

Conclusion

The refactoring successfully reorganized the NVML enum structure to improve readability and maintenance, while maintaining full backward compatibility with the existing codebase.

@miaobyte miaobyte closed this Aug 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

dco-signoff: yes size/XXL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@miaobyte