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

move providers to UMF #1

Merged
merged 1 commit into from
Jul 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion source/common/unified_malloc_framework/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

set(UMF_SOURCES
src/memory_pool.c
src/memory_provider.c
src/memory_provider.cpp
src/memory_tracker.cpp
src/memory_provider_get_last_failed.cpp
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
extern "C" {
#endif

typedef struct umf_memory_provider_t *umf_memory_provider_handle_t;
Copy link
Collaborator

Choose a reason for hiding this comment

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

where it is used?


/// \brief This structure comprises function pointers used by corresponding umfPool*
/// calls. Each memory pool implementation should initialize all function
/// pointers.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,26 @@ typedef struct umf_memory_provider_t *umf_memory_provider_handle_t;
/// \return UMF_RESULT_SUCCESS on success or appropriate error code on failure.
///
enum umf_result_t
umfMemoryProviderCreate(struct umf_memory_provider_ops_t *ops, void *params,
umf_memory_provider_handle_t *hProvider);
umfMemoryProviderCreate(const struct umf_memory_provider_ops_t *ops,
void *params, umf_memory_provider_handle_t *hProvider);

///
/// \brief Destroys memory provider.
/// \param hPool handle to the memory provider
///
void umfMemoryProviderDestroy(umf_memory_provider_handle_t hProvider);

// TODO comment
enum umf_result_t
umfMemoryProviderRegister(struct umf_memory_provider_ops_t *ops);

enum umf_result_t
umfMemoryProvidersRegistryGet(struct umf_memory_provider_ops_t *providers,
size_t *numProviders);

const struct umf_memory_provider_ops_t *
umfMemoryProvidersRegistryGetOps(char *name);
Copy link
Collaborator

Choose a reason for hiding this comment

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

would it be better to return enum umf_result_t as in other APIs and return ops as an output parameter?


///
/// \brief Allocates size bytes of uninitialized storage from memory provider
/// with specified alignment.
Expand Down Expand Up @@ -110,7 +121,7 @@ umfMemoryProviderGetMinPageSize(umf_memory_provider_handle_t hProvider,

///
/// \brief Discard physical pages within the virtual memory mapping associated at given addr and size.
/// This call is asynchronous and may delay puring the pages indefinitely.
/// This call is asynchronous and may delay purging the pages indefinitely.
/// \param hProvider handle to the memory provider
/// \param ptr beginning of the virtual memory range
/// \param size size of the virtual memory range
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#ifndef UMF_MEMORY_PROVIDER_OPS_H
#define UMF_MEMORY_PROVIDER_OPS_H 1

#include <stdbool.h>
#include <umf/base.h>

#ifdef __cplusplus
Expand All @@ -26,16 +27,16 @@ struct umf_memory_provider_ops_t {
uint32_t version;

///
/// \brief Initializes memory pool.
/// \param params pool-specific params
/// \param pool returns pointer to the pool
/// \brief Initializes memory provider.
/// \param params provider-specific params
/// \param provider returns pointer to the provider
/// \return UMF_RESULT_SUCCESS on success or appropriate error code on failure.
enum umf_result_t (*initialize)(void *params, void **pool);
enum umf_result_t (*initialize)(void *params, void **provider);

///
/// \brief Finalizes memory pool.
/// \param pool pool to finalize
void (*finalize)(void *pool);
/// \brief Finalizes memory provider.
/// \param provider provider to finalize
void (*finalize)(void *provider);

/// Refer to memory_provider.h for description of those functions
enum umf_result_t (*alloc)(void *provider, size_t size, size_t alignment,
Expand All @@ -50,6 +51,7 @@ struct umf_memory_provider_ops_t {
enum umf_result_t (*purge_lazy)(void *provider, void *ptr, size_t size);
enum umf_result_t (*purge_force)(void *provider, void *ptr, size_t size);
const char *(*get_name)(void *provider);
bool (*supports_device)(const char *name);
};

#ifdef __cplusplus
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,30 @@
*
*/

#include "memory_provider_internal.h"
#include <umf/memory_provider.h>

#include <assert.h>
#include <stdlib.h>

#include <algorithm>
#include <cstring>
#include <string>
#include <vector>

#include "umf/memory_provider.h"

#include "memory_provider_internal.h"

struct umf_memory_provider_t {
struct umf_memory_provider_ops_t ops;
void *provider_priv;
};

std::vector<struct umf_memory_provider_ops_t> globalProviders;

enum umf_result_t
umfMemoryProviderCreate(struct umf_memory_provider_ops_t *ops, void *params,
umf_memory_provider_handle_t *hProvider) {
umfMemoryProviderCreate(const struct umf_memory_provider_ops_t *ops,
void *params, umf_memory_provider_handle_t *hProvider) {
umf_memory_provider_handle_t provider =
malloc(sizeof(struct umf_memory_provider_t));
(umf_memory_provider_t *)malloc(sizeof(struct umf_memory_provider_t));
if (!provider) {
return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY;
}
Expand All @@ -46,6 +54,42 @@ umfMemoryProviderCreate(struct umf_memory_provider_ops_t *ops, void *params,
return UMF_RESULT_SUCCESS;
}

enum umf_result_t umfMemoryProviderRegister(umf_memory_provider_ops_t *ops) {

// TODO check if this provider isn't already registered
globalProviders.push_back(*ops);

return UMF_RESULT_SUCCESS;
}

enum umf_result_t
umfMemoryProvidersRegistryGet(umf_memory_provider_ops_t *providers,
size_t *numProviders) {

if (providers == NULL) {
*numProviders = globalProviders.size();
} else {
memcpy(providers, globalProviders.data(),
sizeof(umf_memory_provider_ops_t) * *numProviders);
}

return UMF_RESULT_SUCCESS;
}

// TODO rename ;)
const umf_memory_provider_ops_t *umfMemoryProvidersRegistryGetOps(char *name) {
auto it = std::find_if(
std::begin(globalProviders), std::end(globalProviders),
[&](auto &ops) { return std::strcmp(ops.get_name(NULL), name) == 0; });
Copy link
Collaborator

Choose a reason for hiding this comment

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

A general comment: until now we assumed that all function pointers in ops would only be used through the public APIs of memory pool or provider, not accessed directly. This means that, you could assume the first argument (NULL in your case) is never NULL. If that's no longer the case we should probably add some info about this in documentation (for get_name at least)


if (it != globalProviders.end()) {
return &(*it);
}

// else
return NULL;
}

void umfMemoryProviderDestroy(umf_memory_provider_handle_t hProvider) {
hProvider->ops.finalize(hProvider->provider_priv);
free(hProvider);
Expand Down