From bc09a69c7242dd401ada2ceb3fc1c6fdc84ff7b9 Mon Sep 17 00:00:00 2001 From: Jan Christoph Uhde Date: Sat, 27 Aug 2016 03:07:48 +0200 Subject: [PATCH] optimization that skips the body of addPayload used in http case tests - fail --- lib/Rest/GeneralResponse.cpp | 60 +++++++++++++++++++----------------- lib/Rest/GeneralResponse.h | 16 +++++----- lib/Rest/HttpResponse.cpp | 6 ++-- lib/Rest/HttpResponse.h | 7 +++-- 4 files changed, 49 insertions(+), 40 deletions(-) diff --git a/lib/Rest/GeneralResponse.cpp b/lib/Rest/GeneralResponse.cpp index 655a4b0f4c..5728beed6f 100644 --- a/lib/Rest/GeneralResponse.cpp +++ b/lib/Rest/GeneralResponse.cpp @@ -32,46 +32,50 @@ using namespace arangodb::basics; void GeneralResponse::addPayload(VPackSlice const& slice, arangodb::velocypack::Options const* options, - bool resolve_externals) { - addPayloadPreconditions(); - addPayloadPreHook(false, resolve_externals); - if (!options) { + bool resolveExternals) { + if (!options) { // like this because nullptr is easier to pass than + // VPackOptions::Options::Defaults options = &arangodb::velocypack::Options::Defaults; } - - if (resolve_externals) { - auto tmpBuffer = - basics::VelocyPackHelper::sanitizeExternalsChecked(slice, options); - _vpackPayloads.push_back(std::move(tmpBuffer)); - } else { - // just copy - _vpackPayloads.emplace_back(slice.byteSize()); - std::memcpy(&_vpackPayloads.back(), slice.start(), slice.byteSize()); + addPayloadPreconditions(); + bool skipBody = false; + addPayloadPreHook(true, resolveExternals, skipBody); + if (!skipBody) { + if (resolveExternals) { + auto tmpBuffer = + basics::VelocyPackHelper::sanitizeExternalsChecked(slice, options); + _vpackPayloads.push_back(std::move(tmpBuffer)); + } else { + // just copy + _vpackPayloads.emplace_back(slice.byteSize()); + std::memcpy(&_vpackPayloads.back(), slice.start(), slice.byteSize()); + } + addPayloadPostHook(slice, options, resolveExternals); } - addPayloadPostHook(options); }; void GeneralResponse::addPayload(VPackBuffer&& buffer, arangodb::velocypack::Options const* options, - bool resolve_externals) { + bool resolveExternals) { addPayloadPreconditions(); - // TODO - // skip sanatizing here for http if conent type is json because it will - // be dumped anyway -- check with jsteemann - addPayloadPreHook(true, resolve_externals); - if (!options) { options = &arangodb::velocypack::Options::Defaults; } - - if (resolve_externals) { - auto tmpBuffer = basics::VelocyPackHelper::sanitizeExternalsChecked( - VPackSlice(buffer.data()), options); - _vpackPayloads.push_back(std::move(tmpBuffer)); - } else { - _vpackPayloads.push_back(std::move(buffer)); + // TODO + // skip sanatizing here for http if conent type is json because it will + // be dumped anyway -- check with jsteemann + bool skipBody = false; + addPayloadPreHook(true, resolveExternals, skipBody); + if (!skipBody) { + if (resolveExternals) { + auto tmpBuffer = basics::VelocyPackHelper::sanitizeExternalsChecked( + VPackSlice(buffer.data()), options); + _vpackPayloads.push_back(std::move(tmpBuffer)); + } else { + _vpackPayloads.push_back(std::move(buffer)); + } } - addPayloadPostHook(options); + addPayloadPostHook(VPackSlice(buffer.data()), options, resolveExternals); }; std::string GeneralResponse::responseString(ResponseCode code) { diff --git a/lib/Rest/GeneralResponse.h b/lib/Rest/GeneralResponse.h index 7816928f03..d0b3716f0c 100644 --- a/lib/Rest/GeneralResponse.h +++ b/lib/Rest/GeneralResponse.h @@ -129,15 +129,17 @@ class GeneralResponse { virtual int reservePayload(std::size_t size) { return size; } void addPayloadPreconditions() { TRI_ASSERT(_vpackPayloads.size() == 0); }; - virtual void addPayloadPreHook(bool inputIsBuffer, bool& resolveExternals){}; - virtual void addPayloadPostHook( - arangodb::velocypack::Options const* options){}; - void addPayload(VPackSlice const& slice, - arangodb::velocypack::Options const* options = nullptr, + virtual void addPayloadPreHook(bool inputIsBuffer, bool& resolveExternals, + bool& skipBody){}; + void addPayload(VPackSlice const&, + arangodb::velocypack::Options const* = nullptr, bool resolve_externals = true); - void addPayload(VPackBuffer&& buffer, - arangodb::velocypack::Options const* options = nullptr, + void addPayload(VPackBuffer&&, + arangodb::velocypack::Options const* = nullptr, bool resolve_externals = true); + virtual void addPayloadPostHook(VPackSlice const&, + arangodb::velocypack::Options const*, + bool resolveExternals){}; void setOptions(VPackOptions options) { _options = std::move(options); }; diff --git a/lib/Rest/HttpResponse.cpp b/lib/Rest/HttpResponse.cpp index 8c3c980988..89618a3506 100644 --- a/lib/Rest/HttpResponse.cpp +++ b/lib/Rest/HttpResponse.cpp @@ -300,9 +300,9 @@ void HttpResponse::writeHeader(StringBuffer* output) { // end of header, body to follow } -void HttpResponse::addPayloadPostHook(VPackOptions const* options) { - VPackSlice slice(_vpackPayloads.front().data()); - +void HttpResponse::addPayloadPostHook(VPackSlice const& slice, + VPackOptions const* options, + bool resolveExternals) { switch (_contentType) { case rest::ContentType::VPACK: { size_t length = static_cast(slice.byteSize()); diff --git a/lib/Rest/HttpResponse.h b/lib/Rest/HttpResponse.h index dfff8ce16b..d3fd438790 100644 --- a/lib/Rest/HttpResponse.h +++ b/lib/Rest/HttpResponse.h @@ -74,8 +74,10 @@ class HttpResponse : public GeneralResponse { public: void reset(ResponseCode code) override final; - void addPayloadPreHook(bool inputIsBuffer, bool& resolveExternals) override { + void addPayloadPreHook(bool inputIsBuffer, bool& resolveExternals, + bool& skipBody) override { if (_contentType == ContentType::JSON) { + skipBody = true; // resolveExternals = false; // they get resolved during dump in post // hook // this optimization leads to bad bas crahses @@ -83,9 +85,10 @@ class HttpResponse : public GeneralResponse { }; int reservePayload(std::size_t size) override { return _body.reserve(size); } - void addPayloadPostHook(VPackOptions const* options) override; void setPayload(arangodb::velocypack::Slice const&, bool generateBody, arangodb::velocypack::Options const&) override final; + void addPayloadPostHook(VPackSlice const&, VPackOptions const* options, + bool resolveExternals) override; arangodb::Endpoint::TransportType transportType() override { return arangodb::Endpoint::TransportType::HTTP;