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

Split processors into monitor and user #237

Merged
merged 11 commits into from
Mar 5, 2024
Merged

Split processors into monitor and user #237

merged 11 commits into from
Mar 5, 2024

Conversation

Christian-B
Copy link
Member

@Christian-B Christian-B commented Mar 4, 2024

Every single use of chip.processors was inefficient.

Typically the caller want just the ids and/ or just the user cores.

This pr adds extra methods to provide these more efficiently.

To add efficiency in the common case where just user processors (or their ids) are wanted the monitor and user processors are split into two dicts.

This comes at a slight cost for some methods using all processors but not enough to make is desirable to have duplicates dicts.
The only use cases that wanted all processors actually only wanted their ids so the new all_processor_ids method is faster anyway


Chip now also consistently uses machine_version().n_non_user_cores to determine the number of monitor processor and Chips just point to the global dict.
(A pointer to the global dict was required to allow some tests to hack in a different number of monitor cores)

tested by
http://apollo.cs.man.ac.uk:8080/blue/organizations/jenkins/Integration%20Tests/detail/chip_test/2/pipeline
(note that actually tested with Chip.processors completely remove which is why the different branch names)

Fixes #182

@coveralls
Copy link

coveralls commented Mar 4, 2024

Coverage Status

coverage: 93.714% (+0.006%) from 93.708%
when pulling ee9d0ca on split_processors
into ecc3ae3 on master.

@rowleya rowleya merged commit e416aa3 into master Mar 5, 2024
9 checks passed
@rowleya rowleya deleted the split_processors branch March 5, 2024 14:28
This pull request was closed.
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.

Chip and View get_none_monitor cores
3 participants