Skip to content

Fix documentation to mem step size and clean up memory manager test #2821

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

Merged
merged 1 commit into from
Apr 9, 2020

Conversation

jacobkahn
Copy link
Contributor

  • Add documentation to set and get mem step size functions in the C/C++ API to properly reflect behavior if a custom memory manager is set
  • Remove af_event use from unlock in memory management test
  • Ensure memory manager test memory pressure functions and gc threshold more closely matches default memory manager behavior

9prady9
9prady9 previously requested changes Apr 3, 2020
Copy link
Member

@9prady9 9prady9 left a comment

Choose a reason for hiding this comment

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

Can you please fix the formatting, else looks good.

@@ -236,12 +236,14 @@ namespace af
AFAPI void deviceGC();
/// @}

/// \brief Set the resolution of memory chunks
/// \brief Set the resolution of memory chunks. Works only with the default
/// memory manager - throws if a custom memory manager is set.
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -773,7 +769,7 @@ af_err alloc_fn(af_memory_manager manager, void **ptr,
get_memory_pressure_fn(manager, &pressure);
float threshold;
af_memory_manager_get_memory_pressure_threshold(manager, &threshold);
if (pressure > threshold) { signal_memory_cleanup_fn(manager); }
if (pressure >= threshold) { signal_memory_cleanup_fn(manager); }

Copy link
Member

Choose a reason for hiding this comment

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

The changes in this file basically reflect the change in #2801 Is that correct ?

@9prady9 9prady9 added this to the 3.7.2 milestone Apr 8, 2020
@umar456 umar456 dismissed 9prady9’s stale review April 9, 2020 23:08

I will fix the formatting in a future commit

@umar456 umar456 merged commit 3cb51ab into arrayfire:master Apr 9, 2020
@umar456 umar456 mentioned this pull request Jun 27, 2020
2 tasks
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.

3 participants