Skip to content
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.

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

Windows is adding new Relationship values to LOGICAL_PROCESSOR_RELATIONSHIP enum #1324

Closed
dbwiddis opened this issue Mar 11, 2021 · 5 comments · Fixed by #1563
Closed

Windows is adding new Relationship values to LOGICAL_PROCESSOR_RELATIONSHIP enum #1324

dbwiddis opened this issue Mar 11, 2021 · 5 comments · Fixed by #1563
Assignees
Labels
documentation Updating site docs or javadocs

Comments

@dbwiddis
Copy link
Contributor

The existing implementation of Kernel32Util.getLogicalProcessorInformationEx() assumes that only the values 0 through 4 are possible values of the Relationship field of SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX based on existing documentation.

However, a value of 7 has been observed using Insider preview versions of Windows 10: See oshi/oshi#1554

Interestingly, the SYSTEM_LOGICAL_PROCESSOR_INFORMATION docs (which included only enum values 0,1,2,3) include this tidbit:

Future versions of Windows may support additional values for the Relationship member.

That caveat didn't extend to the _EX version but it's apparently true. The latest published SDK headers (build 20279) include values 5 and 6:

typedef enum _LOGICAL_PROCESSOR_RELATIONSHIP {
    RelationProcessorCore,
    RelationNumaNode,
    RelationCache,
    RelationProcessorPackage,
    RelationGroup,
    RelationProcessorDie,
    RelationNumaNodeEx,
    RelationAll = 0xffff
} LOGICAL_PROCESSOR_RELATIONSHIP;

This introduces a future compatibility problem because we currently throw an exception for values other than the first five in the list.

There are still only four possible mappings in the union, and the mystery enum value 7 appears to align with the PROCESSOR_RELATIONSHIP structure. One would guess that works for 5 as well, and 6 would map to the NUMA_NODE_RELATIONSHIP.

Opening this now as a reminder to track this. Not sure if we want to guess at the new mappings, but I think it might be reasonable to allow values 5, 6, and 7 to just be skipped in the iteration rather than throwing an exception.

@dbwiddis
Copy link
Contributor Author

Complete new version of the enum:

typedef enum _LOGICAL_PROCESSOR_RELATIONSHIP {
    RelationProcessorCore,
    RelationNumaNode,
    RelationCache,
    RelationProcessorPackage,
    RelationGroup,
    RelationProcessorDie,
    RelationNumaNodeEx,
    RelationProcessorModule,
    RelationAll = 0xffff
} LOGICAL_PROCESSOR_RELATIONSHIP;

@dbwiddis
Copy link
Contributor Author

Other changes based on insider headers:

  • NUMA_NODE_RELATIONSHIP has only 18 bytes reserved instead of 20. Last two bytes are a short GroupCount and the array is now variable sized.
  • RelationNumaNodeEx maps to NUMA_NODE_RELATIONSHIP
  • RelationProcessorDie and RelationshipProcessorModule map to PROCESSOR_RELATIONSHIP (similar to/alternatives for Package on some architectures)

@dbwiddis
Copy link
Contributor Author

dbwiddis commented Jun 2, 2021

The online documentation for LOGICAL_PROCESSOR_RELATIONSHIP now includes the first version of the header in the original post. Interestingly, the RelationNumaNodeEx is documented but RelationProcessorDie, which precedes it in the enumeration, is not mentioned yet.

The NUMA_NODE_RELATIONSHIP header has been updated with the new group information this new enum value enables. I'll submit a PR to update this.

@dbwiddis
Copy link
Contributor Author

The changes in #1363 should work for all the new code logic I've seen. I'll leave this issue open to track updating the javadocs once the Windows docs are finalized.

@dbwiddis
Copy link
Contributor Author

Looks like the docs are finally up to date. I'll submit a PR this weekend to fix up the javadocs and finally close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Updating site docs or javadocs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant