Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/topic/robin/revert-deferred'
Browse files Browse the repository at this point in the history
* origin/topic/robin/revert-deferred:
  Remove `std::functional` from `DeferredExpression`.
  Revert "Remove support for deferred expressions."
  • Loading branch information
rsmmr committed May 13, 2024
2 parents 1ace1fa + 2f513be commit f523256
Show file tree
Hide file tree
Showing 13 changed files with 304 additions and 1 deletion.
57 changes: 57 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
@@ -1,3 +1,60 @@
1.11.0-dev.160 | 2024-05-13 11:45:04 +0200

* GH-1657: Update Spicy runtime driver to use new stream features for improved performance. (Robin Sommer, Corelight)

This does two things:

- When adding data to a stream, we now do that without copying
anything initially. For block input (e.g., UDP) that's always fine
because the parser will never suspend before it's fully done
parsing; hence we can safely delete it once the parser returns. For
stream input (e.g., TCP), we make the stream own its data later
if (and only if) the parser suspends.

- For block input (e.g., UDP) we now keep reusing the same stream for
subsequent blocks, instead of creating a new one each time. This
allows the stream to reuse an allocated chunk that it may have still
cached internally.

The result of this, plus the new chunk caching introduced earlier, is
that for a UDP flow, we never need to allocate more than one chunk,
and never need to copy any data; and for TCP it's the same as long as
parsing consumes all data before suspending (which should be a common
case), plus, when we allocate new storage we only copy data that didn't
get trimmed immediately anyways.

* Give stream a method to reset it into freshly initialized state. (Robin Sommer, Corelight)

This does not clear the internal chunk cache.

* Cache previously trimmed chunks inside stream for reuse. (Robin Sommer, Corelight)

A chain now retains one previously used but no longer needed chunk for
reuse, so that we can avoid constant cycles of creating/destructing
chunks (and their paylaod memory) in the common case of a parser
consuming full chunks without yielding. The caching is also geared
towards owning/non-owning semantics staying consistent across
subsequent append operations.

* Extend stream API to allow for chunks that don't own their data. (Robin Sommer, Corelight)

By default, we still copy data when creating chunks but we add a
parallel API that just stores pointers, assuming the data will stay
around as long as needed. If the stream owner cannot guarantee that,
they may at any point convert all not-owned data into owned data
through a corresponding `makeOwning()` stream method. To make that
method efficient even with long chains of chunks, we internally
maintain an invariant that only the last chunk of chain can be
non-owning: whenever we add a new chunk to a chain, we ensure that the
previous tail become owning at that point. In other words, we amortize
the work across all newly added chunks.

* Remove `std::functional` from `DeferredExpression`. (Robin Sommer)

* Revert "Remove support for deferred expressions." (Robin Sommer)
Turns out this is actually still being used by the Zeek
integration.

1.11.0-dev.151 | 2024-05-10 16:35:50 +0200

* Remove unused include headers. (Robin Sommer, Corelight)
Expand Down
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.11.0-dev.151
1.11.0-dev.160
1 change: 1 addition & 0 deletions hilti/runtime/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ add_executable(
src/tests/bytes.cc
src/tests/context.cc
src/tests/debug-logger.cc
src/tests/deferred-expression.cc
src/tests/enum.cc
src/tests/exception.cc
src/tests/fiber.cc
Expand Down
64 changes: 64 additions & 0 deletions hilti/runtime/include/deferred-expression.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// Copyright (c) 2020-2023 by the Zeek Project. See LICENSE for details.

#pragma once

#include <string>
#include <utility>

#include <hilti/rt/extension-points.h>
#include <hilti/rt/types/result.h>

namespace hilti::rt {

/**
* Wrapper for an expression that's evaluation is deferred until requested.
* The expression must be wrapped into a function call, and it's evaluation
* is requested through the wrapper's call operator.
*
* The function should be stateless as it might be invoked an unspecified
* number of times.
*/
template<typename Result, typename Expr>
class DeferredExpression {
public:
DeferredExpression(Expr&& expr) : _expr(std::move(expr)) {}
DeferredExpression() = delete;
DeferredExpression(const DeferredExpression&) = default;
DeferredExpression(DeferredExpression&&) noexcept = default;

~DeferredExpression() = default;

DeferredExpression& operator=(const DeferredExpression&) = default;
DeferredExpression& operator=(DeferredExpression&&) noexcept = default;

Result operator()() const { return _expr(); }

private:
Expr _expr;
};

template<typename Result, typename Expr>
auto make_deferred(Expr&& expr) {
return DeferredExpression<Result, Expr>(std::forward<Expr>(expr));
}

namespace detail::adl {
template<typename Result, typename Expr>
inline std::string to_string(const DeferredExpression<Result, Expr>& x, adl::tag /*unused*/) {
return hilti::rt::to_string(x());
}
} // namespace detail::adl

// This function is declared as an overload since we cannot partially specialize
// `hilti::detail::to_string_for_print` for `DeferredExpression<T, Expr>`.
template<typename Result, typename Expr>
inline std::string to_string_for_print(const DeferredExpression<Result, Expr>& x) {
return hilti::rt::to_string_for_print(x());
}

template<typename Result, typename Expr>
inline std::ostream& operator<<(std::ostream& out, const DeferredExpression<Result, Expr>& x) {
return out << to_string_for_print(x);
}

} // namespace hilti::rt
1 change: 1 addition & 0 deletions hilti/runtime/include/libhilti.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <hilti/rt/autogen/version.h>
#include <hilti/rt/configuration.h>
#include <hilti/rt/context.h>
#include <hilti/rt/deferred-expression.h>
#include <hilti/rt/exception.h>
#include <hilti/rt/extension-points.h>
#include <hilti/rt/fiber-check-stack.h>
Expand Down
96 changes: 96 additions & 0 deletions hilti/runtime/src/tests/deferred-expression.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
// Copyright (c) 2020-2023 by the Zeek Project. See LICENSE for details.

#include <cstddef>
#include <type_traits>

#include <hilti/rt/deferred-expression.h>
#include <hilti/rt/doctest.h>
#include <hilti/rt/types/integer.h>

using namespace hilti::rt;

TEST_SUITE_BEGIN("DeferredExpression");

TEST_CASE("assign") {
int i = 0;
auto expr = make_deferred<int32_t>([&]() { return ++i; });
REQUIRE_EQ(i, 0);

CHECK_EQ(expr(), 1);
CHECK_EQ(i, 1);

SUBCASE("rvalue") {
auto expr = make_deferred<int32_t>([]() { return 0; });
CHECK_EQ(expr(), 0);
CHECK_EQ(i, 1); // Not incrementing anymore.
}
}

TEST_CASE("construct") {
int i = 0;
auto expr = make_deferred<int>([&i]() { return ++i; });

SUBCASE("default") {
// Construction does not evaluate passed function.
CHECK_EQ(i, 0);
}

SUBCASE("copy") {
auto expr2 = DeferredExpression(expr);
// Copy construction does not evaluate passed function.
CHECK_EQ(i, 0);

// Copies share any data dependencies of original function.
REQUIRE_EQ(expr(), 1);
CHECK_EQ(i, 1);

REQUIRE_EQ(expr2(), 2);
CHECK_EQ(i, 2);
}

SUBCASE("move") {
auto expr2 = DeferredExpression(std::move(expr));
// Move construction does not evaluate passed function.
CHECK_EQ(i, 0);

REQUIRE_EQ(expr2(), 1);
CHECK_EQ(i, 1);
}
}

TEST_CASE("evaluate") {
int i = 0;
auto expr = make_deferred<int>([&i]() { return ++i; });

CHECK_EQ(expr(), 1);
CHECK_EQ(expr(), 2);
}

TEST_CASE("fmt") {
int i = 0;
auto expr = make_deferred<int>([&i]() { return ++i; });

// Stringification evaluates the expression.
CHECK_EQ(fmt("%s", expr), "1");
CHECK_EQ(fmt("%s", expr), "2");
}

TEST_CASE("to_string") {
int i = 0;
auto expr = make_deferred<int>([&i]() { return ++i; });

// Stringification evaluates the expression.
CHECK_EQ(to_string(expr), "1");
CHECK_EQ(to_string(expr), "2");
}

TEST_CASE("to_string_for_print") {
int i = 0;
auto expr = make_deferred<Bytes>([&i]() { return Bytes(fmt("\\x0%d", ++i)); });

// Stringification evaluates the expression.
CHECK_EQ(to_string_for_print(expr), R"#(\\x01)#");
CHECK_EQ(to_string_for_print(expr), R"#(\\x02)#");
}

TEST_SUITE_END();
2 changes: 2 additions & 0 deletions hilti/toolchain/include/ast/builder/builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,8 @@ class Builder : public builder::NodeFactory {
return expressionUnresolvedOperator(operator_::Kind::SumAssign, {op1, op2}, m);
}

auto deferred(Expression* e, const Meta& m = Meta()) { return expressionDeferred(e, m); }

auto differenceAssign(Expression* op1, Expression* op2, const Meta& m = Meta()) {
return expressionUnresolvedOperator(operator_::Kind::DifferenceAssign, {op1, op2}, m);
}
Expand Down
6 changes: 6 additions & 0 deletions hilti/toolchain/include/ast/builder/node-factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,12 @@ class NodeFactory {
auto expressionCtor(Ctor* ctor, Meta meta = {}) {
return hilti::expression::Ctor::create(context(), ctor, std::move(meta));
}
auto expressionDeferred(Expression* expr, bool catch_exception, const Meta& meta = {}) {
return hilti::expression::Deferred::create(context(), expr, catch_exception, meta);
}
auto expressionDeferred(Expression* expr, const Meta& meta = {}) {
return hilti::expression::Deferred::create(context(), expr, meta);
}
auto expressionGrouping(Expression* expr, Meta meta = {}) {
return hilti::expression::Grouping::create(context(), expr, std::move(meta));
}
Expand Down
1 change: 1 addition & 0 deletions hilti/toolchain/include/ast/expressions/all.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <hilti/ast/expressions/builtin-function.h>
#include <hilti/ast/expressions/coerced.h>
#include <hilti/ast/expressions/ctor.h>
#include <hilti/ast/expressions/deferred.h>
#include <hilti/ast/expressions/grouping.h>
#include <hilti/ast/expressions/keyword.h>
#include <hilti/ast/expressions/list-comprehension.h>
Expand Down
51 changes: 51 additions & 0 deletions hilti/toolchain/include/ast/expressions/deferred.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright (c) 2020-2023 by the Zeek Project. See LICENSE for details.

#pragma once

#include <memory>
#include <utility>

#include <hilti/ast/expression.h>
#include <hilti/ast/type.h>

namespace hilti::expression {

/**
* AST node for an expression for which evaluation is deferred at runtime to
* a later point when explicitly requested by the runtime system. Optionally,
* that later evaluation can catch any exceptions and return a corresponding
* ``result<T>``.
*/
class Deferred : public Expression {
public:
auto expression() const { return child<Expression>(0); }
bool catchException() const { return _catch_exception; }

QualifiedType* type() const final { return child<QualifiedType>(1); }

node::Properties properties() const final {
auto p = node::Properties{{"catch_exception", _catch_exception}};
return Expression::properties() + p;
}

void setType(ASTContext* ctx, QualifiedType* t) { setChild(ctx, 1, t); }

static auto create(ASTContext* ctx, Expression* expr, bool catch_exception, const Meta& meta = {}) {
return ctx->make<Deferred>(ctx, {expr, QualifiedType::createAuto(ctx, meta)}, catch_exception, meta);
}

static auto create(ASTContext* ctx, Expression* expr, const Meta& meta = {}) {
return create(ctx, expr, false, meta);
}

protected:
Deferred(ASTContext* ctx, Nodes children, bool catch_exception, Meta meta)
: Expression(ctx, NodeTags, std::move(children), std::move(meta)), _catch_exception(catch_exception) {}

HILTI_NODE_1(expression::Deferred, Expression, final);

private:
bool _catch_exception;
};

} // namespace hilti::expression
1 change: 1 addition & 0 deletions hilti/toolchain/include/ast/visitor-dispatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ class Dispatcher {
virtual void operator()(hilti::expression::BuiltInFunction*) {}
virtual void operator()(hilti::expression::Coerced*) {}
virtual void operator()(hilti::expression::Ctor*) {}
virtual void operator()(hilti::expression::Deferred*) {}
virtual void operator()(hilti::expression::Grouping*) {}
virtual void operator()(hilti::expression::Keyword*) {}
virtual void operator()(hilti::expression::ListComprehension*) {}
Expand Down
15 changes: 15 additions & 0 deletions hilti/toolchain/src/compiler/codegen/expressions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,21 @@ struct Visitor : hilti::visitor::PreOrder {

void operator()(expression::Ctor* n) final { result = cg->compile(n->ctor(), lhs); }

void operator()(expression::Deferred* n) final {
auto type = cg->compile(n->type(), codegen::TypeUsage::Storage);
auto value = cg->compile(n->expression());

if ( n->catchException() )
// We can't pass the exception through here, so we just return a
// default constructed return value.
result =
fmt("::hilti::rt::make_deferred<%s>([=]() -> %s { try { return %s; } catch ( ... ) { return "
"{}; } })",
type, type, value);
else
result = fmt("::hilti::rt::make_deferred<%s>([=]() -> %s { return %s; })", type, type, value);
}

void operator()(expression::Grouping* n) final { result = fmt("(%s)", cg->compile(n->expression(), lhs)); }

void operator()(expression::Keyword* n) final {
Expand Down
8 changes: 8 additions & 0 deletions hilti/toolchain/src/compiler/resolver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <hilti/ast/declarations/imported-module.h>
#include <hilti/ast/declarations/local-variable.h>
#include <hilti/ast/declarations/parameter.h>
#include <hilti/ast/expressions/deferred.h>
#include <hilti/ast/expressions/keyword.h>
#include <hilti/ast/expressions/list-comprehension.h>
#include <hilti/ast/expressions/name.h>
Expand Down Expand Up @@ -1141,6 +1142,13 @@ struct VisitorPass2 : visitor::MutatingPostOrder {
}
}

void operator()(expression::Deferred* n) final {
if ( ! n->type()->isResolved() && n->expression()->isResolved() ) {
recordChange(n, n->expression()->type());
n->setType(context(), n->expression()->type());
}
}

void operator()(expression::Keyword* n) final {
if ( n->kind() == expression::keyword::Kind::Scope && ! n->type()->isResolved() ) {
auto ntype = builder()->qualifiedType(builder()->typeString(), Constness::Const);
Expand Down

0 comments on commit f523256

Please sign in to comment.