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

add memusage stat to os_provider and use it in benchmarks #1149

Merged
merged 1 commit into from
Mar 12, 2025

Conversation

lplewa
Copy link
Contributor

@lplewa lplewa commented Mar 3, 2025

Description

Checklist

  • Code compiles without errors locally
  • All tests pass locally
  • CI workflows execute properly
  • CI workflows, not executed per PR (e.g. Nightly), execute properly
  • New tests added, especially if they will fail without my changes
  • Added/extended example(s) to cover this functionality
  • Extended the README/documentation
  • All newly added source files have a license
  • All newly added source files are referenced in CMake files
  • Logger (with debug/info/... messages) is used
  • All API changes are reflected in docs and def/map files, and are tested

atomic_fetch_add_explicit(&os_provider->stats.allocated_memory, size,
memory_order_relaxed);

size_t allocated = atomic_load_explicit(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need to load os_provider->stats.allocated_memory again, instead of using the result of the atomic_fetch_add_explicit on the line above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@lplewa lplewa force-pushed the stats branch 6 times, most recently from 1d11852 to 8950beb Compare March 6, 2025 16:40
@lplewa lplewa marked this pull request as ready for review March 6, 2025 16:40
@lplewa lplewa requested a review from a team as a code owner March 6, 2025 16:40
@lplewa lplewa force-pushed the stats branch 5 times, most recently from 9dfb5ed to 0d7e960 Compare March 7, 2025 16:27
Comment on lines 321 to 324
memused.resize(state.threads());
for (int i = 0; i < state.threads(); i++) {
memused[i] = 0;
}
Copy link
Contributor

@KFilipek KFilipek Mar 10, 2025

Choose a reason for hiding this comment

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

Suggested change
memused.resize(state.threads());
for (int i = 0; i < state.threads(); i++) {
memused[i] = 0;
}
memused.assign(state.threads(), 0);

or

Suggested change
memused.resize(state.threads());
for (int i = 0; i < state.threads(); i++) {
memused[i] = 0;
}
memused.resize(state.threads());
std::fill(memused.begin(), memused.end(), 0);

size_t current_memory_allocated = 0;
for (int i = 0; i < state.threads(); i++) {
current_memory_allocated += memused[i];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
size_t current_memory_allocated = 0;
for (int i = 0; i < state.threads(); i++) {
current_memory_allocated += memused[i];
}
size_t current_memory_allocated = 0;
for (const auto &used : memused) {
current_memory_allocated += used;
}

@lplewa lplewa force-pushed the stats branch 2 times, most recently from 45bdf01 to 2504e25 Compare March 11, 2025 13:02
@bratpiorka bratpiorka merged commit f326630 into oneapi-src:v0.12.x-dev Mar 12, 2025
78 checks passed
KFilipek pushed a commit to KFilipek/unified-memory-framework that referenced this pull request Mar 17, 2025
add memusage stat to os_provider and use it in benchmarks
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.

4 participants