tursom/memgraph
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Cache globally allocated mgp_type instances

Browse Source 
Summary:
This simplifies the C API and reduces total allocations done when
constructing a type.

Reviewers: mferencevic, ipaljak

Reviewed By: mferencevic

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D2550
This commit is contained in:
Teon Banek 2019-11-14 15:53:44 +01:00
parent d17d463884
commit 1dc97a37f8
5 changed files with 72 additions and 141 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
include/mg_procedure.h
View File

@ -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"

3
src/query/procedure/cypher_types.hpp
View File

@ -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));
});
}

113
src/query/procedure/mg_procedure_impl.cpp
View File

@ -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;
}
}

2
src/query/procedure/mg_procedure_impl.hpp
View File

@ -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};
};

39
tests/unit/query_procedure_mgp_type.cpp
View File

@ -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);
}
}