diff --git a/include/mg_procedure.h b/include/mg_procedure.h index 088da8e5a..88a11e7c3 100644 --- a/include/mg_procedure.h +++ b/include/mg_procedure.h @@ -587,42 +587,32 @@ const struct mgp_vertex *mgp_vertices_iterator_next( /// procedure signature for use with openCypher. Memgraph will use the built /// procedure signature to perform various static and dynamic checks when the /// custom procedure is invoked. -/// -/// Building a type may perform allocations, so you need to release the instance -/// with mgp_type_destroy. For easier usage, all of the API which takes -/// non-const `struct mgp_type *` as an argument will take the ownership, so you -/// don't have to call mgp_type_destroy on such arguments. In most cases, you -/// will only need to release mgp_type that is the final instance which you -/// haven't passed to a function. ///@{ /// A type for values in openCypher. struct mgp_type; -/// Free the memory used by the given mgp_type instance. -void mgp_type_destroy(struct mgp_type *type); - /// Get the type representing any value that isn't `null`. /// /// The ANY type is the parent type of all types. -struct mgp_type *mgp_type_any(); +const struct mgp_type *mgp_type_any(); /// Get the type representing boolean values. -struct mgp_type *mgp_type_bool(); +const struct mgp_type *mgp_type_bool(); /// Get the type representing character string values. -struct mgp_type *mgp_type_string(); +const struct mgp_type *mgp_type_string(); /// Get the type representing integer values. -struct mgp_type *mgp_type_int(); +const struct mgp_type *mgp_type_int(); /// Get the type representing floating-point values. -struct mgp_type *mgp_type_float(); +const struct mgp_type *mgp_type_float(); /// Get the type representing any number value. /// /// This is the parent type for numeric types, i.e. INTEGER and FLOAT. -struct mgp_type *mgp_type_number(); +const struct mgp_type *mgp_type_number(); /// Get the type representing map values. /// @@ -633,50 +623,32 @@ struct mgp_type *mgp_type_number(); /// /// @sa mgp_type_node /// @sa mgp_type_relationship -struct mgp_type *mgp_type_map(); +const struct mgp_type *mgp_type_map(); /// Get the type representing graph node values. /// /// Since a node contains a map of properties, the node itself is also of MAP /// type. -struct mgp_type *mgp_type_node(); +const struct mgp_type *mgp_type_node(); /// Get the type representing graph relationship values. /// /// Since a relationship contains a map of properties, the relationship itself /// is also of MAP type. -struct mgp_type *mgp_type_relationship(); +const struct mgp_type *mgp_type_relationship(); /// Get the type representing a graph path (walk) from one node to another. -struct mgp_type *mgp_type_path(); +const struct mgp_type *mgp_type_path(); /// Build a type representing a list of values of given `element_type`. /// -/// `element_type` will be transferred to the new instance, and you must not use -/// `element_type` after the function successfully returns. -/// -/// Instantiating this type will perform an allocation. If you do not give -/// ownership of the returned instance to someone else, you need to release the -/// memory explicitly using mgp_type_destroy. -/// -/// NULL is returned if unable to allocate the new type. `element_type` is -/// intact in such a case, so you will need to call mgp_type_destroy on it. -struct mgp_type *mgp_type_list(struct mgp_type *element_type, - struct mgp_memory *memory); +/// NULL is returned if unable to allocate the new type. +const struct mgp_type *mgp_type_list(const struct mgp_type *element_type); /// Build a type representing either a `null` value or a value of given `type`. /// -/// `type` will be transferred to the new instance, and you must not use `type` -/// after the function successfully returns. -/// -/// Instantiating this type will perform an allocation. If you do not give -/// ownership of the instance to someone else, you need to release the memory -/// explicitly using mgp_type_destroy. -/// -/// NULL is returned if unable to allocate the new type. `type` is intact in -/// such a case, so you will need to call mgp_type_destroy on it. -struct mgp_type *mgp_type_nullable(struct mgp_type *type, - struct mgp_memory *memory); +/// NULL is returned if unable to allocate the new type. +const struct mgp_type *mgp_type_nullable(const struct mgp_type *type); ///@} #ifdef __cplusplus } // extern "C" diff --git a/src/query/procedure/cypher_types.hpp b/src/query/procedure/cypher_types.hpp index 85f1434ef..c5dd099b3 100644 --- a/src/query/procedure/cypher_types.hpp +++ b/src/query/procedure/cypher_types.hpp @@ -158,8 +158,7 @@ class NullableType : public CypherType { alloc.deallocate(nullable, 1); throw; } - return CypherTypePtr(nullable, [memory](CypherType *base_ptr) { - utils::Allocator<NullableType> alloc(memory); + return CypherTypePtr(nullable, [alloc](CypherType *base_ptr) mutable { alloc.delete_object(static_cast<NullableType *>(base_ptr)); }); } diff --git a/src/query/procedure/mg_procedure_impl.cpp b/src/query/procedure/mg_procedure_impl.cpp index e8699cfbe..fa487c4c9 100644 --- a/src/query/procedure/mg_procedure_impl.cpp +++ b/src/query/procedure/mg_procedure_impl.cpp @@ -1220,140 +1220,115 @@ const mgp_vertex *mgp_vertices_iterator_next(mgp_vertices_iterator *it) { } /// Type System +/// +/// All types are allocated globally, so that we simplify the API and minimize +/// allocations done for types. namespace { void NoOpCypherTypeDeleter(CypherType *) {} } // namespace -void mgp_type_destroy(mgp_type *type) { - if (!type || !type->memory) return; - utils::Allocator<mgp_type> alloc(type->memory); - alloc.delete_object(type); -} - -mgp_type *mgp_type_any() { +const mgp_type *mgp_type_any() { static AnyType impl; static mgp_type any_type{CypherTypePtr(&impl, NoOpCypherTypeDeleter)}; return &any_type; } -mgp_type *mgp_type_bool() { +const mgp_type *mgp_type_bool() { static BoolType impl; static mgp_type bool_type{CypherTypePtr(&impl, NoOpCypherTypeDeleter)}; return &bool_type; } -mgp_type *mgp_type_string() { +const mgp_type *mgp_type_string() { static StringType impl; static mgp_type string_type{CypherTypePtr(&impl, NoOpCypherTypeDeleter)}; return &string_type; } -mgp_type *mgp_type_int() { +const mgp_type *mgp_type_int() { static IntType impl; static mgp_type int_type{CypherTypePtr(&impl, NoOpCypherTypeDeleter)}; return &int_type; } -mgp_type *mgp_type_float() { +const mgp_type *mgp_type_float() { static FloatType impl; static mgp_type float_type{CypherTypePtr(&impl, NoOpCypherTypeDeleter)}; return &float_type; } -mgp_type *mgp_type_number() { +const mgp_type *mgp_type_number() { static NumberType impl; static mgp_type number_type{CypherTypePtr(&impl, NoOpCypherTypeDeleter)}; return &number_type; } -mgp_type *mgp_type_map() { +const mgp_type *mgp_type_map() { static MapType impl; static mgp_type map_type{CypherTypePtr(&impl, NoOpCypherTypeDeleter)}; return &map_type; } -mgp_type *mgp_type_node() { +const mgp_type *mgp_type_node() { static NodeType impl; static mgp_type node_type{CypherTypePtr(&impl, NoOpCypherTypeDeleter)}; return &node_type; } -mgp_type *mgp_type_relationship() { +const mgp_type *mgp_type_relationship() { static RelationshipType impl; static mgp_type relationship_type{ CypherTypePtr(&impl, NoOpCypherTypeDeleter)}; return &relationship_type; } -mgp_type *mgp_type_path() { +const mgp_type *mgp_type_path() { static PathType impl; static mgp_type path_type{CypherTypePtr(&impl, NoOpCypherTypeDeleter)}; return &path_type; } -mgp_type *mgp_type_list(mgp_type *type, mgp_memory *memory) { - utils::Allocator<mgp_type> alloc(memory->impl); - // We allocate seperately, because we want to correctly release the passed in - // `type` w.r.t. to exceptions. This way if our allocation fails, nothing - // happens to `type`. But when we take ownership of it below with - // alloc.new_object<ListType>, then we need to make sure that everything after - // that point is exception-free so that `type` is released. - mgp_type *list_type; +const mgp_type *mgp_type_list(const mgp_type *type) { + // Maps `type` to corresponding instance of ListType. + static utils::pmr::map<const mgp_type *, mgp_type> list_types( + utils::NewDeleteResource()); + static utils::SpinLock lock; + std::lock_guard<utils::SpinLock> guard(lock); + auto found_it = list_types.find(type); + if (found_it != list_types.end()) return &found_it->second; try { - list_type = alloc.allocate(1); + auto alloc = list_types.get_allocator(); + CypherTypePtr impl( + alloc.new_object<ListType>( + // Just obtain the pointer to original impl, don't own it. + CypherTypePtr(type->impl.get(), NoOpCypherTypeDeleter), + alloc.GetMemoryResource()), + [alloc](CypherType *base_ptr) mutable { + alloc.delete_object(static_cast<ListType *>(base_ptr)); + }); + return &list_types.emplace(type, mgp_type{std::move(impl)}).first->second; } catch (const std::bad_alloc &) { return nullptr; } - try { - auto *impl = alloc.new_object<ListType>( - type->memory ? std::move(type->impl) - // It would be an error to move the globally allocated - // mgp_type, instead just copy the pointer. - : CypherTypePtr(type->impl.get(), NoOpCypherTypeDeleter), - memory->impl); - // Everything below should not throw anything. - new (list_type) mgp_type{ - CypherTypePtr(impl, - [memory](CypherType *base_ptr) { - utils::Allocator<ListType> alloc(memory->impl); - alloc.delete_object(static_cast<ListType *>(base_ptr)); - }), - memory->impl}; - mgp_type_destroy(type); - return list_type; - } catch (const std::bad_alloc &) { - alloc.deallocate(list_type, 1); - return nullptr; - } } -mgp_type *mgp_type_nullable(mgp_type *type, mgp_memory *memory) { - utils::Allocator<mgp_type> alloc(memory->impl); - // We allocate seperately, because we want to correctly release the passed in - // `type` w.r.t. to exceptions. This way if our allocation fails, nothing - // happens to `type`. But when we take ownership of it below with - // NullableType::Create, then we need to make sure that everything after that - // point is exception-free so that `type` is released. - mgp_type *nullable_type; - try { - nullable_type = alloc.allocate(1); - } catch (const std::bad_alloc &) { - return nullptr; - } +const mgp_type *mgp_type_nullable(const mgp_type *type) { + // Maps `type` to corresponding instance of NullableType. + static utils::pmr::map<const mgp_type *, mgp_type> gNullableTypes( + utils::NewDeleteResource()); + static utils::SpinLock lock; + std::lock_guard<utils::SpinLock> guard(lock); + auto found_it = gNullableTypes.find(type); + if (found_it != gNullableTypes.end()) return &found_it->second; try { + auto alloc = gNullableTypes.get_allocator(); auto impl = NullableType::Create( - type->memory ? std::move(type->impl) - // It would be an error to move the globally allocated - // mgp_type, instead just copy the pointer. - : CypherTypePtr(type->impl.get(), NoOpCypherTypeDeleter), - memory->impl); - // Everything below should not throw anything. - new (nullable_type) mgp_type{std::move(impl), memory->impl}; - mgp_type_destroy(type); - return nullable_type; + CypherTypePtr(type->impl.get(), NoOpCypherTypeDeleter), + alloc.GetMemoryResource()); + return &gNullableTypes.emplace(type, mgp_type{std::move(impl)}) + .first->second; } catch (const std::bad_alloc &) { - alloc.deallocate(nullable_type, 1); return nullptr; } } diff --git a/src/query/procedure/mg_procedure_impl.hpp b/src/query/procedure/mg_procedure_impl.hpp index 382dc8fce..510eb8406 100644 --- a/src/query/procedure/mg_procedure_impl.hpp +++ b/src/query/procedure/mg_procedure_impl.hpp @@ -459,6 +459,4 @@ struct mgp_vertices_iterator { struct mgp_type { query::procedure::CypherTypePtr impl; - // Optional for globally allocated mgp_type. - utils::MemoryResource *memory{nullptr}; }; diff --git a/tests/unit/query_procedure_mgp_type.cpp b/tests/unit/query_procedure_mgp_type.cpp index acfa5a621..86ef0c174 100644 --- a/tests/unit/query_procedure_mgp_type.cpp +++ b/tests/unit/query_procedure_mgp_type.cpp @@ -17,55 +17,42 @@ TEST(CypherType, PresentableNameSimpleTypes) { } TEST(CypherType, PresentableNameCompositeTypes) { - mgp_memory memory{utils::NewDeleteResource()}; { - auto *nullable_any = mgp_type_nullable(mgp_type_any(), &memory); + const auto *nullable_any = mgp_type_nullable(mgp_type_any()); EXPECT_EQ(nullable_any->impl->GetPresentableName(), "ANY?"); - mgp_type_destroy(nullable_any); } { - auto *nullable_any = - mgp_type_nullable(mgp_type_nullable(mgp_type_any(), &memory), &memory); + const auto *nullable_any = + mgp_type_nullable(mgp_type_nullable(mgp_type_any())); EXPECT_EQ(nullable_any->impl->GetPresentableName(), "ANY?"); - mgp_type_destroy(nullable_any); } { - auto *nullable_list = - mgp_type_nullable(mgp_type_list(mgp_type_any(), &memory), &memory); + const auto *nullable_list = + mgp_type_nullable(mgp_type_list(mgp_type_any())); EXPECT_EQ(nullable_list->impl->GetPresentableName(), "LIST? OF ANY"); - mgp_type_destroy(nullable_list); } { - auto *list_of_int = mgp_type_list(mgp_type_int(), &memory); + const auto *list_of_int = mgp_type_list(mgp_type_int()); EXPECT_EQ(list_of_int->impl->GetPresentableName(), "LIST OF INTEGER"); - mgp_type_destroy(list_of_int); } { - auto *list_of_nullable_path = - mgp_type_list(mgp_type_nullable(mgp_type_path(), &memory), &memory); + const auto *list_of_nullable_path = + mgp_type_list(mgp_type_nullable(mgp_type_path())); EXPECT_EQ(list_of_nullable_path->impl->GetPresentableName(), "LIST OF PATH?"); - mgp_type_destroy(list_of_nullable_path); } { - auto *list_of_list_of_map = - mgp_type_list(mgp_type_list(mgp_type_map(), &memory), &memory); + const auto *list_of_list_of_map = + mgp_type_list(mgp_type_list(mgp_type_map())); EXPECT_EQ(list_of_list_of_map->impl->GetPresentableName(), "LIST OF LIST OF MAP"); - mgp_type_destroy(list_of_list_of_map); } { - auto *nullable_list_of_nullable_list_of_nullable_string = mgp_type_nullable( - mgp_type_list( - mgp_type_nullable( - mgp_type_list(mgp_type_nullable(mgp_type_string(), &memory), - &memory), - &memory), - &memory), - &memory); + const auto *nullable_list_of_nullable_list_of_nullable_string = + mgp_type_nullable(mgp_type_list(mgp_type_nullable( + mgp_type_list(mgp_type_nullable(mgp_type_string()))))); EXPECT_EQ(nullable_list_of_nullable_list_of_nullable_string->impl ->GetPresentableName(), "LIST? OF LIST? OF STRING?"); - mgp_type_destroy(nullable_list_of_nullable_list_of_nullable_string); } }