[WIP] mem props #1301
Draft
[WIP] mem props #1301
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bratpiorka
force-pushed
the
rrudnick_props_api
branch
15 times, most recently
from
92db71d to
56573df
Compare
bratpiorka
force-pushed
the
rrudnick_props_api
branch
15 times, most recently
from
a5c357c to
17f6335
Compare
bratpiorka
force-pushed
the
rrudnick_props_api
branch
15 times, most recently
from
96bb935 to
5015432
Compare
lplewa
requested changes
Jun 12, 2025
benchmark/ubench.c
Outdated
Comment on lines
442
to
465
| static void do_umf_mem_props_benchmark(ze_context_handle_t context, | ||
| bool use_umf, alloc_t *allocs, | ||
| size_t num_allocs, size_t repeats) { | ||
| assert(context != NULL); | ||
|
|
||
| for (size_t r = 0; r < repeats * 10; ++r) { | ||
| for (size_t i = 0; i < num_allocs; ++i) { | ||
| if (use_umf) { | ||
| umf_memory_properties_handle_t props_handle = NULL; | ||
| umf_result_t res = | ||
| umfGetMemoryPropertiesHandle(allocs[i].ptr, &props_handle); | ||
| (void)res; | ||
| assert(res == UMF_RESULT_SUCCESS); | ||
|
|
||
| umf_usm_memory_type_t type = UMF_MEMORY_TYPE_UNKNOWN; | ||
| res = umfGetMemoryProperty( | ||
| props_handle, UMF_MEMORY_PROPERTY_POINTER_TYPE, &type); | ||
| assert(res == UMF_RESULT_SUCCESS); | ||
| if (type != UMF_MEMORY_TYPE_DEVICE) { | ||
| fprintf(stderr, | ||
| "error: unexpected alloc_props.type value: %d\n", | ||
| type); | ||
| exit(-1); |
There was a problem hiding this comment.
Please in final version add benchmark to new framework.
include/umf/memory_props.h
Outdated
Comment on lines
31
to
32
| /// be filled. NOTE: the type and size of the value depends on the | ||
| /// memory property ID and should be checked in the documentation |
There was a problem hiding this comment.
api like this should include size parameter for safety
Comment on lines
+19
to
+25
| /// @brief Get the memory properties handle for a given pointer | ||
| /// @param ptr pointer to the allocated memory | ||
| /// @param props_handle [out] pointer to the memory properties handle | ||
| /// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure | ||
| umf_result_t | ||
| umfGetMemoryPropertiesHandle(const void *ptr, | ||
| umf_memory_properties_handle_t *props_handle); |
There was a problem hiding this comment.
document ownership of this handle.
Comment on lines
+28
to
+36
| typedef struct umf_memory_properties_t { | ||
| // TODO alloc_info_t | ||
| void *ptr; | ||
| umf_memory_pool_handle_t pool; | ||
| umf_memory_provider_handle_t provider; | ||
| uint64_t id; | ||
| void *base; | ||
| size_t base_size; | ||
| } umf_memory_properties_t; |
There was a problem hiding this comment.
if we have api to literaly read this structure - it should be defined in .c file
| #include <umf/memory_provider.h> | ||
|
|
||
| #if UMF_BUILD_LEVEL_ZERO_PROVIDER | ||
| #include "ze_api.h" |
There was a problem hiding this comment.
i do not see a reason to include this file there
| /// @param value [out] pointer to the preallocated variable where the value will be stored | ||
| /// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure, | ||
| /// UMF_RESULT_ERROR_INVALID_ARGUMENT if memory_property_id is invalid or value is NULL, | ||
| /// UMF_RESULT_ERROR_NOT_SUPPORTED if the property is not supported by this provider. |
There was a problem hiding this comment.
This is not true:
Please include tests to ensure that comment it true (as far i see we return EINVAL
There was a problem hiding this comment.
Especially please include also test with custom user provided provider, which support extra property which is user defined.
bratpiorka
force-pushed
the
rrudnick_props_api
branch
12 times, most recently
from
daa3c56 to
96e5b38
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
todo