8000 [APM-662] Improve memory usage (Expressions, AqlNamePath) (#19405) · arangodb/arangodb@dffd752 · GitHub
[go: up one dir, main page]
Skip to content

Commit dffd752

Browse files
hkernbachjsteemann
hkernbach
and
jsteemann
authored
[APM-662] Improve memory usage (Expressions, AqlNamePath) (#19405)
* improve mem tracking * remove unused function, add mem tracking for projections * more tracking * clang format * just a test * next attempt * fix dummy code * clang format * properly init _vertexShards * properly pass allocator * adjust comment * rollback some changes as they do not work * prepare Expression for resourceMonitor to be able to track memory here * attempt to fix ci failures * clang format * this should not be required * take care of mem use with parallelism * remove logging * init commit for expressions and aqlnamepaths * fix issues found by tests * fix tests * Apply suggestions from code review Remove not needed comments / cod e * Update arangod/Graph/BaseOptions.cpp * Update arangod/Graph/BaseOptions.h * clang format * Update arangod/Graph/BaseOptions.cpp Co-authored-by: Jan <jsteemann@users.noreply.github.com> * use std::move * code review: try catch * documentFastPathLocal now uses std::string_view instead of std::string const& - this change unfortunately leads to a lot of additional function signature changes * clang format * clang format * memtrack AttributeNamePath * move _usedBytesByData out of the union * code review: do not use a scope guard if we do have a try catch block already. directly clear structs after decrease of memory * more use of absl::StrCat, prevent string copy where not required * fix typo * prevent string copy. Implementation now support std::string_view * adjust code to code conventions * remove not required includes to speed up compilation * re-establish default constructor (deleted) * prevent string copy in CollLevelMap * stick to code conventions * fix build, fix wrong projections build up in the EdgeCursor * fix wrong projections build up in the EdgeCursor again * fix wrong projections build up in the pregel based EdgeCursor ... again * fix default constructor in AttributeNamePath, re-establish default deleted constructor * fix test binary build * do NOT use current to get the resMonitor reference - as current ptr is changed dynamically and runs into nullptr * extended memory tracking usage * do not create a vector of strings - use string_view instead - also removed some not required includes * reset _usedBytesByData in error case * clang format * clang format * move helper method into anonymous namespace * attempt to fix windows build * Revert " attempt to fix windows build" This reverts commit c80999c. * attempt to fix windows build #2 * fix false positive cppcheck * do not inplace build std::vector of string_views * clang format * move p instead of rebuilding it * simpler constructor * clangormat --------- Co-authored-by: Jan <jsteemann@users.noreply.github.com>
1 parent 1d2ef3d commit dffd752

35 files changed

+738
-433
lines changed

‎arangod/Aql/AqlValue.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,15 @@
2323

2424
#include "AqlValue.h"
2525

26-
#include "Aql/AqlItemBlock.h"
2726
#include "Aql/Arithmetic.h"
2827
#include "Aql/Range.h"
29-
#include "Aql/SharedAqlItemBlockPtr.h"
3028
#include "Basics/Endian.h"
3129
#include "Basics/VelocyPackHelper.h"
3230
#include "Transaction/Context.h"
3331
#include "Transaction/Helpers.h"
34-
#include "Transaction/Methods.h"
3532
#include "V8/v8-vpack.h"
3633

3734
#include <velocypack/Buffer.h>
38-
#include <velocypack/Iterator.h>
3935
#include <velocypack/Slice.h>
4036
#include <type_traits>
4137

@@ -725,7 +721,7 @@ AqlValue AqlValue::get(CollectionNameResolver const& resolver,
725721

726722
/// @brief get the (object) element(s) by name
727723
AqlValue AqlValue::get(CollectionNameResolver const& resolver,
728-
std::vector<std::string> const& names, bool& mustDestroy,
724+
MonitoredStringVector const& names, bool& mustDestroy,
729725
bool doCopy) const {
730726
mustDestroy = false;
731727
if (names.empty()) {

‎arangod/Aql/AqlValue.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "Aql/AqlValueFwd.h"
2828
#include "Aql/types.h"
2929
#include "Basics/Endian.h"
30+
#include "Basics/MemoryTypes/MemoryTypes.h"
3031
#include "IResearch/Misc.h"
3132

3233
#include <velocypack/Slice.h>
@@ -419,7 +420,7 @@ struct AqlValue final {
419420
AqlValue get(CollectionNameResolver const& resolver, std::string_view name,
420421
bool& mustDestroy, bool copy) const;
421422
AqlValue get(CollectionNameResolver const& resolver,
422-
std::vector<std::string> const& names, bool& mustDestroy,
423+
MonitoredStringVector const& names, bool& mustDestroy,
423424
bool copy) const;
424425
bool hasKey(std::string_view name) const;
425426

‎arangod/Aql/Ast.cpp

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,9 @@ auto doNothingVisitor = [](AstNode const*) {};
105105
/// AttributeFinderContext to a single AttributePath, and injects it into the
106106
/// attributeNamePath within the Context.
107107
/// Returns true if and only if the context found an attribute access path.
108-
bool translateNodeStackToAttributePath(RecursiveAttributeFinderContext& state) {
108+
bool translateNodeStackToAttributePath(
109+
RecursiveAttributeFinderContext& state,
110+
arangodb::ResourceMonitor& resourceMonitor) {
109111
TRI_ASSERT(!state.seen.empty());
110112
AstNode const* top = state.seen.back();
111113
TRI_ASSERT(top->type == NODE_TYPE_REFERENCE);
@@ -176,7 +178,9 @@ bool translateNodeStackToAttributePath(RecursiveAttributeFinderContext& state) {
176178
}
177179

178180
// now take off all projections from the stack
179-
std::vector<std::string> path;
181+
ResourceUsageAllocator<MonitoredStringVector, ResourceMonitor> alloc = {
182+
resourceMonitor};
183+
MonitoredStringVector path{alloc};
180184
while (top->type == NODE_TYPE_ATTRIBUTE_ACCESS) {
181185
path.emplace_back(top->getString());
182186
state.seen.pop_back();
@@ -187,7 +191,7 @@ bool translateNodeStackToAttributePath(RecursiveAttributeFinderContext& state) {
187191
}
188192

189193
TRI_ASSERT(!path.empty());
190-
state.attributes.emplace(std::move(path));
194+
state.attributes.emplace(AttributeNamePath(std::move(path)));
191195
}
192196

193197
return true;
@@ -2772,7 +2776,8 @@ size_t Ast::countReferences(AstNode const* node, Variable const* search) {
27722776
bool Ast::getReferencedAttributesRecursive(
27732777
AstNode const* node, Variable const* variable,
27742778
std::string_view expectedAttribute,
2775-
containers::FlatHashSet<aql::AttributeNamePath>& attributes) {
2779+
containers::FlatHashSet<aql::AttributeNamePath>& attributes,
2780+
arangodb::ResourceMonitor& resourceMonitor) {
27762781
RecursiveAttributeFinderContext state{variable,
27772782
/*couldExtractAttributePath*/ true,
27782783
expectedAttribute,
@@ -2846,7 +2851,7 @@ bool Ast::getReferencedAttributesRecursive(
28462851
state.seen.push_back(iterRhs);
28472852
// finally the stack is complete... now eval it to find the
28482853
// projection
2849-
if (!::translateNodeStackToAttributePath(state)) {
2854+
if (!::translateNodeStackToAttributePath(state, resourceMonitor)) {
28502855
state.couldExtractAttributePath = false;
28512856
}
28522857
}
@@ -2870,7 +2875,7 @@ bool Ast::getReferencedAttributesRecursive(
28702875
// reference to a variable
28712876
state.seen.push_back(node);
28722877
// evaluate whatever we have on the stack
2873-
if (!translateNodeStackToAttributePath(state)) {
2878+
if (!translateNodeStackToAttributePath(state, resourceMonitor)) {
28742879
state.couldExtractAttributePath = false;
28752880
}
28762881
// fall-through intentional

‎arangod/Aql/Ast.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
#include "Aql/VariableGenerator.h"
4343
#include "Aql/types.h"
4444
#include "Basics/AttributeNameParser.h"
45+
#include "Basics/ResourceUsage.h"
4546
#include "Containers/FlatHashSet.h"
4647
#include "Containers/HashSet.h"
4748
#include "Graph/PathType.h"
@@ -475,7 +476,8 @@ class Ast {
475476
/// for the specified variable
476477
static bool getReferencedAttributesRecursive(
477478
AstNode const*, Variable const*, std::string_view expectedAttribute,
478-
containers::FlatHashSet<aql::AttributeNamePath>&);
479+
containers::FlatHashSet<aql::AttributeNamePath>&,
480+
arangodb::ResourceMonitor& resourceMonitor);
479481

480482
/// @brief replace an attribute access with just the variable
481483
static AstNode* replaceAttributeAccess(

‎arangod/Aql/AttributeAccessor.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ class AttributeAccessorTo final : public AttributeAccessor {
8989
class AttributeAccessorSingle final : public AttributeAccessor {
9090
public:
9191
explicit AttributeAccessorSingle(Variable const* variable,
92-
std::string const& attributeName)
92+
std::string_view attributeName)
9393
: AttributeAccessor(variable), _attributeName(attributeName) {}
9494

9595
AqlValue get(CollectionNameResolver const& resolver,
@@ -115,7 +115,7 @@ class AttributeAccessorMulti final : public AttributeAccessor {
115115
AqlValue const& value =
116116
context->getVariableValue(_variable, false, mustDestroy);
117117
// use general version for multiple attributes (e.g. variable.attr.subattr)
118-
return value.get(resolver, _path.get(), mustDestroy, true);
118+
return value.get(resolver, _path._path, mustDestroy, true);
119119
}
120120

121121
private:

‎arangod/Aql/AttributeNamePath.cpp

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
////////////////////////////////////////////////////////////////////////////////
2323

2424
#include "Aql/AttributeNamePath.h"
25+
#include "Basics/MemoryTypes/MemoryTypes.h"
2526
#include "Basics/StaticStrings.h"
2627
#include "Basics/debugging.h"
2728
#include "Basics/fasthash.h"
@@ -32,29 +33,37 @@
3233
namespace arangodb {
3334
namespace aql {
3435

35-
AttributeNamePath::AttributeNamePath(std::string attribute) {
36-
path.emplace_back(std::move(attribute));
36+
AttributeNamePath::AttributeNamePath(
37+
arangodb::ResourceMonitor& resourceMonitor) noexcept
38+
: _path(ResourceUsageAllocator<MonitoredStringVector, ResourceMonitor>{
39+
resourceMonitor}) {}
40+
41+
AttributeNamePath::AttributeNamePath(std::string attribute,
42+
arangodb::ResourceMonitor& resourceMonitor)
43+
: _path(ResourceUsageAllocator<MonitoredStringVector, ResourceMonitor>{
44+
resourceMonitor}) {
45+
_path.emplace_back(std::move(attribute));
3746
}
3847

39-
AttributeNamePath::AttributeNamePath(std::vector<std::string> p)
40-
: path(std::move(p)) {}
48+
AttributeNamePath::AttributeNamePath(MonitoredStringVector p) noexcept
49+
: _path(std::move(p)) {}
4150

42-
bool AttributeNamePath::empty() const noexcept { return path.empty(); }
51+
bool AttributeNamePath::empty() const noexcept { return _path.empty(); }
4352

44-
size_t AttributeNamePath::size() const noexcept { return path.size(); }
53+
size_t AttributeNamePath::size() const noexcept { return _path.size(); }
4554

4655
AttributeNamePath::Type AttributeNamePath::type() const noexcept {
4756
TRI_ASSERT(!empty());
4857

4958
Type type = Type::SingleAttribute;
5059
if (size() == 1) {
51-
if (path[0] == StaticStrings::IdString) {
60+
if (_path[0] == std::string_view{StaticStrings::IdString}) {
5261
type = Type::IdAttribute;
53-
} else if (path[0] == StaticStrings::KeyString) {
62+
} else if (_path[0] == std::string_view{StaticStrings::KeyString}) {
5463
type = Type::KeyAttribute;
55-
} else if (path[0] == StaticStrings::FromString) {
64+
} else if (_path[0] == std::string_view{StaticStrings::FromString}) {
5665
type = Type::FromAttribute;
57-
} else if (path[0] == StaticStrings::ToString) {
66+
} else if (_path[0] == std::string_view{StaticStrings::ToString}) {
5867
type = Type::ToAttribute;
5968
}
6069
} else {
@@ -69,24 +78,24 @@ size_t AttributeNamePath::hash() const noexcept {
6978
// platform-dependent. however, we need portable hash values because we are
7079
// testing for them in unit tests.
7180
uint64_t hash = 0x0404b00b1e5;
72-
for (auto const& it : path) {
81+
for (auto const& it : _path) {
7382
hash = fasthash64(it.data(), it.size(), hash);
7483
}
7584
return static_cast<size_t>(hash);
7685
}
7786

78-
std::string const& AttributeNamePath::operator[](size_t index) const noexcept {
79-
TRI_ASSERT(index < path.size());
80-
return path[index];
87+
std::string_view AttributeNamePath::operator[](size_t index) const noexcept {
88+
TRI_ASSERT(index < _path.size());
89+
return std::string_view{_path[index]};
8190
}
8291

8392
bool AttributeNamePath::operator==(
8493
AttributeNamePath const& other) const noexcept {
85-
if (path.size() != other.path.size()) {
94+
if (_path.size() != other._path.size()) {
8695
return false;
8796
}
88-
for (size_t i = 0; i < path.size(); ++i) {
89-
if (path[i] != other.path[i]) {
97+
for (size_t i = 0; i < _path.size(); ++i) {
98+
if (_path[i] != other._path[i]) {
9099
return false;
91100
}
92101
}
@@ -97,23 +106,23 @@ bool AttributeNamePath::operator<(
97106
AttributeNamePath const& other) const noexcept {
98107
size_t const commonLength = std::min(size(), other.size());
99108
for (size_t i = 0; i < commonLength; ++i) {
100-
if (path[i] < other[i]) {
109+
if (_path[i] < other[i]) {
101110
return true;
102-
} else if (path[i] > other[i]) {
111+
} else if (_path[i] > other[i]) {
103112
return false;
104113
}
105114
}
106115
return (size() < other.size());
107116
}
108117

109-
std::vector<std::string> const& AttributeNamePath::get() const noexcept {
110-
return path;
118+
MonitoredStringVector const& AttributeNamePath::get() const noexcept {
119+
return _path;
111120
}
112121

113-
void AttributeNamePath::clear() noexcept { path.clear(); }
122+
void AttributeNamePath::clear() noexcept { _path.clear(); }
114123

115124
AttributeNamePath& AttributeNamePath::reverse() {
116-
std::reverse(path.begin(), path.end());
125+
std::reverse(_path.begin(), _path.end());
117126
return *this;
118127
}
119128

@@ -122,7 +131,7 @@ AttributeNamePath& AttributeNamePath::shortenTo(size_t length) {
122131
if (length >= size()) {
123132
return *this;
124133
}
125-
path.erase(path.begin() + length, path.end());
134+
_path.erase(_path.begin() + length, _path.end());
126135
return *this;
127136
}
128137

@@ -141,8 +150,8 @@ AttributeNamePath& AttributeNamePath::shortenTo(size_t length) {
141150

142151
std::ostream& operator<<(std::ostream& stream, AttributeNamePath const& path) {
143152
stream << "[";
144-
for (auto const& it : path.path) {
145-
stream << ' ' << it;
153+
for (auto const& it : path._path) {
154+
stream << ' ' << std::string_view{it};
146155
}
147156
stream << " ]";
148157
return stream;

‎arangod/Aql/AttributeNamePath.h

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,12 @@
2323

2424
#pragma once
2525

26+
#include "Basics/MemoryTypes/MemoryTypes.h"
27+
2628
#include <cstdint>
2729
#include <functional>
2830
#include <iosfwd>
2931
#include <string>
30-
#include <vector>
3132

3233
namespace arangodb::aql {
3334

@@ -45,13 +46,15 @@ struct AttributeNamePath {
4546
MultiAttribute // sub-attribute, e.g. a.b.c
4647
};
4748

48-
AttributeNamePath() noexcept {}
49+
explicit AttributeNamePath(
50+
arangodb::ResourceMonitor& resourceMonitor) noexcept;
4951

5052
/// @brief construct an attribute path from a single attribute (e.g. _key)
51-
AttributeNamePath(std::string attribute);
53+
AttributeNamePath(std::string attribute,
54+
arangodb::ResourceMonitor& resourceMonitor);
5255

5356
/// @brief construct an attribute path from a nested attribute (e.g. a.b.c)
54-
AttributeNamePath(std::vector<std::string> path);
57+
explicit AttributeNamePath(MonitoredStringVector path) noexcept;
5558

5659
AttributeNamePath(AttributeNamePath const& other) = default;
5760
AttributeNamePath& operator=(AttributeNamePath const& other) = default;
@@ -71,7 +74,7 @@ struct AttributeNamePath {
7174
size_t hash() const noexcept;
7275

7376
/// @brief get attribute at level
74-
std::string const& operator[](size_t index) const noexcept;
77+
std::string_view operator[](size_t index) const noexcept;
7578

7679
bool operator==(AttributeNamePath const& other) const noexcept;
7780
bool operator!=(AttributeNamePath const& other) const noexcept {
@@ -81,7 +84,7 @@ struct AttributeNamePath {
8184
bool operator<(AttributeNamePath const& other) const noexcept;
8285

8386
/// @brief get the full path
84-
std::vector<std::string> const& get() const noexcept;
87+
[[nodiscard]] MonitoredStringVector const& get() const noexcept;
8588

8689
/// @brief clear all path attributes
8790
void clear() noexcept;
@@ -96,7 +99,7 @@ struct AttributeNamePath {
9699
static size_t commonPrefixLength(AttributeNamePath const& lhs,
97100
AttributeNamePath const& rhs);
98101

99-
std::vector<std::string> path;
102+
MonitoredStringVector _path;
100103
};
101104

102105
std::ostream& operator<<(std::ostream& stream, AttributeNamePath const& path);

‎arangod/Aql/DocumentProducingNode.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "Aql/Variable.h"
3232
#include "Basics/StaticStrings.h"
3333
#include "Basics/VelocyPackHelper.h"
34+
#include "QueryContext.h"
3435

3536
#include <velocypack/Builder.h>
3637
#include <velocypack/Iterator.h>
@@ -51,9 +52,11 @@ DocumentProducingNode::DocumentProducingNode(ExecutionPlan* plan,
5152
arangodb::velocypack::Slice slice)
5253
: _outVariable(
5354
Variable::varFromVPack(plan->getAst(), slice, "outVariable")),
54-
_projections(arangodb::aql::Projections::fromVelocyPack(slice)),
55+
_projections(arangodb::aql::Projections::fromVelocyPack(
56+
slice, plan->getAst()->query().resourceMonitor())),
5557
_filterProjections(arangodb::aql::Projections::fromVelocyPack(
56-
slice, "filterProjections")),
58+
slice, "filterProjections",
59+
plan->getAst()->query().resourceMonitor())),
5760
_count(false),
5861
_useCache(true),
5962
_maxProjections(kMaxProjections) {

0 commit comments

Comments
 (0)
0