From cab9c51669963f52f48568d74cb7fcd6a4bf8d45 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Tue, 3 Dec 2019 20:56:22 +0100 Subject: [PATCH 1/4] Remove unnecessary boost include --- src/mp/proxy.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/mp/proxy.cpp b/src/mp/proxy.cpp index 69231562..8b0c2b02 100644 --- a/src/mp/proxy.cpp +++ b/src/mp/proxy.cpp @@ -11,7 +11,6 @@ #include #include -#include #include #include #include From 5724a2c58a53506f7dd5d6d00b0695b79fb31020 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Tue, 3 Dec 2019 20:58:17 +0100 Subject: [PATCH 2/4] Remove boost usage from GetAnnotation() Split GetAnnotation() to 3 functions that emit the result via an output parameter and return true/false to designate whether it was set or not. --- src/mp/gen.cpp | 84 +++++++++++++++++++++++++++----------------------- 1 file changed, 46 insertions(+), 38 deletions(-) diff --git a/src/mp/gen.cpp b/src/mp/gen.cpp index 4873d0a5..10d6d135 100644 --- a/src/mp/gen.cpp +++ b/src/mp/gen.cpp @@ -25,12 +25,38 @@ constexpr uint64_t NAME_ANNOTATION_ID = 0xb594888f63f4dbb9ull; // From prox constexpr uint64_t SKIP_ANNOTATION_ID = 0x824c08b82695d8ddull; // From proxy.capnp template -boost::optional GetAnnotation(const Reader& reader, uint64_t id) +static bool AnnotationExists(const Reader& reader, uint64_t id) { for (const auto annotation : reader.getAnnotations()) { - if (annotation.getId() == id) return annotation.getValue(); + if (annotation.getId() == id) { + return true; + } + } + return false; +} + +template +static bool GetAnnotationText(const Reader& reader, uint64_t id, kj::StringPtr* result) +{ + for (const auto annotation : reader.getAnnotations()) { + if (annotation.getId() == id) { + *result = annotation.getValue().getText(); + return true; + } + } + return false; +} + +template +static bool GetAnnotationInt32(const Reader& reader, uint64_t id, int32_t* result) +{ + for (const auto annotation : reader.getAnnotations()) { + if (annotation.getId() == id) { + *result = annotation.getValue().getInt32(); + return true; + } } - return {}; + return false; } using CharSlice = kj::ArrayPtr; @@ -162,9 +188,7 @@ void Generate(kj::StringPtr src_prefix, h << "namespace mp {\n"; kj::StringPtr message_namespace; - if (auto value = GetAnnotation(file_schema.getProto(), NAMESPACE_ANNOTATION_ID)) { - message_namespace = value->getText(); - } + GetAnnotationText(file_schema.getProto(), NAMESPACE_ANNOTATION_ID, &message_namespace); std::string base_name = include_base; size_t output_slash = base_name.rfind("/"); @@ -202,9 +226,7 @@ void Generate(kj::StringPtr src_prefix, kj::StringPtr node_name = node_nested.getName(); const auto& node = file_schema.getNested(node_name); kj::StringPtr proxied_class_type; - if (auto proxy = GetAnnotation(node.getProto(), WRAP_ANNOTATION_ID)) { - proxied_class_type = proxy->getText(); - } + GetAnnotationText(node.getProto(), WRAP_ANNOTATION_ID, &proxied_class_type); if (node.getProto().isStruct()) { const auto& struc = node.asStruct(); @@ -239,7 +261,7 @@ void Generate(kj::StringPtr src_prefix, dec << " using Accessors = std::tuple<"; size_t i = 0; for (const auto field : struc.getFields()) { - if (GetAnnotation(field.getProto(), SKIP_ANNOTATION_ID)) { + if (AnnotationExists(field.getProto(), SKIP_ANNOTATION_ID)) { continue; } if (i) dec << ", "; @@ -258,14 +280,12 @@ void Generate(kj::StringPtr src_prefix, inl << " using Struct = " << message_namespace << "::" << node_name << ";\n"; size_t i = 0; for (const auto field : struc.getFields()) { - if (GetAnnotation(field.getProto(), SKIP_ANNOTATION_ID)) { + if (AnnotationExists(field.getProto(), SKIP_ANNOTATION_ID)) { continue; } auto field_name = field.getProto().getName(); auto member_name = field_name; - if (auto name = GetAnnotation(field.getProto(), NAME_ANNOTATION_ID)) { - member_name = name->getText(); - } + GetAnnotationText(field.getProto(), NAME_ANNOTATION_ID, &member_name); inl << " static auto get(std::integral_constant) -> AUTO_RETURN(" << "&" << proxied_class_type << "::" << member_name << ")\n"; ++i; @@ -300,9 +320,7 @@ void Generate(kj::StringPtr src_prefix, for (const auto method : interface.getMethods()) { kj::StringPtr method_name = method.getProto().getName(); kj::StringPtr proxied_method_name = method_name; - if (auto name = GetAnnotation(method.getProto(), NAME_ANNOTATION_ID)) { - proxied_method_name = name->getText(); - } + GetAnnotationText(method.getProto(), NAME_ANNOTATION_ID, &proxied_method_name); const std::string method_prefix = Format() << message_namespace << "::" << node_name << "::" << Cap(method_name); @@ -326,7 +344,7 @@ void Generate(kj::StringPtr src_prefix, bool has_result = false; auto add_field = [&](const ::capnp::StructSchema::Field& schema_field, bool param) { - if (GetAnnotation(schema_field.getProto(), SKIP_ANNOTATION_ID)) { + if (AnnotationExists(schema_field.getProto(), SKIP_ANNOTATION_ID)) { return; } @@ -343,32 +361,22 @@ void Generate(kj::StringPtr src_prefix, has_result = true; } - if (auto value = GetAnnotation(schema_field.getProto(), EXCEPTION_ANNOTATION_ID)) { - field.exception = value->getText(); - } + GetAnnotationText(schema_field.getProto(), EXCEPTION_ANNOTATION_ID, &field.exception); - boost::optional count; - if (auto value = GetAnnotation(schema_field.getProto(), COUNT_ANNOTATION_ID)) { - count = value->getInt32(); - } else if (schema_field.getType().isStruct()) { - if (auto value = - GetAnnotation(schema_field.getType().asStruct().getProto(), COUNT_ANNOTATION_ID)) { - count = value->getInt32(); - } - } else if (schema_field.getType().isInterface()) { - if (auto value = - GetAnnotation(schema_field.getType().asInterface().getProto(), COUNT_ANNOTATION_ID)) { - count = value->getInt32(); + int32_t count = 1; + if (!GetAnnotationInt32(schema_field.getProto(), COUNT_ANNOTATION_ID, &count)) { + if (schema_field.getType().isStruct()) { + GetAnnotationInt32(schema_field.getType().asStruct().getProto(), + COUNT_ANNOTATION_ID, &count); + } else if (schema_field.getType().isInterface()) { + GetAnnotationInt32(schema_field.getType().asInterface().getProto(), + COUNT_ANNOTATION_ID, &count); } } if (inserted.second && !field.retval && !field.exception.size()) { - if (count) { - field.args = *count; - } else { - field.args = 1; - } + field.args = count; } }; From 138ad6794adfd64245d1c8446a5ab494ad38eebf Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Fri, 6 Dec 2019 20:45:31 +0100 Subject: [PATCH 3/4] Change Field::(param and result) to not use boost Fall back to the KISS boolean flag denoting whether the variable is set or not. --- src/mp/gen.cpp | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/mp/gen.cpp b/src/mp/gen.cpp index 10d6d135..9aba915c 100644 --- a/src/mp/gen.cpp +++ b/src/mp/gen.cpp @@ -5,12 +5,13 @@ #include #include -#include +#include #include #include #include #include #include +#include #include #define PROXY_BIN "mpgen" @@ -329,8 +330,10 @@ void Generate(kj::StringPtr src_prefix, struct Field { - boost::optional<::capnp::StructSchema::Field> param; - boost::optional<::capnp::StructSchema::Field> result; + ::capnp::StructSchema::Field param; + bool param_is_set = false; + ::capnp::StructSchema::Field result; + bool result_is_set = false; int args = 0; bool retval = false; bool optional = false; @@ -354,7 +357,13 @@ void Generate(kj::StringPtr src_prefix, fields.emplace_back(); } auto& field = fields[inserted.first->second]; - (param ? field.param : field.result) = schema_field; + if (param) { + field.param = schema_field; + field.param_is_set = true; + } else { + field.result = schema_field; + field.result_is_set = true; + } if (!param && field_name == "result") { field.retval = true; @@ -393,7 +402,7 @@ void Generate(kj::StringPtr src_prefix, fields[field.second].optional = true; } auto want_field = field_idx.find("want" + Cap(field.first)); - if (want_field != field_idx.end() && fields[want_field->second].param) { + if (want_field != field_idx.end() && fields[want_field->second].param_is_set) { fields[want_field->second].skip = true; fields[field.second].requested = true; } @@ -416,12 +425,12 @@ void Generate(kj::StringPtr src_prefix, for (const auto& field : fields) { if (field.skip) continue; - auto field_name = field.param ? field.param->getProto().getName() : - field.result ? field.result->getProto().getName() : ""; - auto field_type = field.param ? field.param->getType() : field.result->getType(); + const auto& f = field.param_is_set ? field.param : field.result; + auto field_name = f.getProto().getName(); + auto field_type = f.getType(); std::ostringstream field_flags; - field_flags << (!field.param ? "FIELD_OUT" : field.result ? "FIELD_IN | FIELD_OUT" : "FIELD_IN"); + field_flags << (!field.param_is_set ? "FIELD_OUT" : field.result_is_set ? "FIELD_IN | FIELD_OUT" : "FIELD_IN"); if (field.optional) field_flags << " | FIELD_OPTIONAL"; if (field.requested) field_flags << " | FIELD_REQUESTED"; if (BoxedType(field_type)) field_flags << " | FIELD_BOXED"; From fb73b81b984bf602ce2f2eaa0f25da2ae252c9e9 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Wed, 4 Dec 2019 11:39:17 +0100 Subject: [PATCH 4/4] Change EventLoop::m_task_set to not use boost EventLoop::m_task_set was unnecessary defined as boost::optional because it is always initialized. It can, however, possibly be destroyed twice: in the EventLoop::loop() method (conditional) and in the EventLoop destructor when it goes out of scope (unconditional). So, use std::unique_ptr to handle this. --- include/mp/proxy-io.h | 3 ++- src/mp/proxy.cpp | 7 +++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/include/mp/proxy-io.h b/include/mp/proxy-io.h index 1e27f96f..e74a5ee0 100644 --- a/include/mp/proxy-io.h +++ b/include/mp/proxy-io.h @@ -15,6 +15,7 @@ #include #include +#include #include namespace mp { @@ -199,7 +200,7 @@ class EventLoop LoggingErrorHandler m_error_handler{*this}; //! Capnp list of pending promises. - boost::optional m_task_set; + std::unique_ptr m_task_set; //! List of connections. std::list m_incoming_connections; diff --git a/src/mp/proxy.cpp b/src/mp/proxy.cpp index 8b0c2b02..e1ae9b84 100644 --- a/src/mp/proxy.cpp +++ b/src/mp/proxy.cpp @@ -123,13 +123,16 @@ void Connection::addAsyncCleanup(std::function fn) } EventLoop::EventLoop(const char* exe_name, LogFn log_fn, void* context) - : m_exe_name(exe_name), m_io_context(kj::setupAsyncIo()), m_log_fn(std::move(log_fn)), m_context(context) + : m_exe_name(exe_name), + m_io_context(kj::setupAsyncIo()), + m_log_fn(std::move(log_fn)), + m_context(context), + m_task_set(new kj::TaskSet(m_error_handler)) { int fds[2]; KJ_SYSCALL(socketpair(AF_UNIX, SOCK_STREAM, 0, fds)); m_wait_fd = fds[0]; m_post_fd = fds[1]; - m_task_set.emplace(m_error_handler); } EventLoop::~EventLoop()