Use std::shared_ptr for variable resolution

AnchoredSetVariable::resolve is called for every rule
(see RuleWithOperator::evaluate). The previous implementation allocated
a new copy of every variable, which quickly added up. In my tests,
AnchoredSetVariable::resolve function consumed 7.8% of run time.

AnchoredSetVariable (which is a multimap) values are never changed,
only added. This means it's safe to store them in std::shared_ptr,
and make resolve return shared_ptr pointing to the same object.

Other resolve implementation could also use this optimization by not
allocating new objects, however, they are not hot spots, so this
optimization was not implemented there.

In my benchmark, this raises performance from 117 requests per second to
131 RPS, and overhead is lowered from 7.8% to 2.4%.

As a bonus, replacing plain pointer with smart pointers make code
cleaner, since using smart pointers makes manual deletes no longer necessary.

Additionally, VariableOrigin is now stored in plain std::vector,
since it's wasteful to store structure containing just two integer
values using std::list<std::unique_ptr<T>>.
This commit is contained in:
WGH
2020-07-28 18:46:03 +03:00
committed by Felipe Zimmerle
parent 7d4cb31b72
commit 1b13e041ce
55 changed files with 264 additions and 376 deletions

View File

@@ -72,10 +72,9 @@ struct MyHash{
class AnchoredSetVariable : public std::unordered_multimap<std::string,
VariableValue *, MyHash, MyEqual> {
std::shared_ptr<const VariableValue>, MyHash, MyEqual> {
public:
AnchoredSetVariable(Transaction *t, const std::string &name);
~AnchoredSetVariable();
void unset();
@@ -93,18 +92,18 @@ class AnchoredSetVariable : public std::unordered_multimap<std::string,
void setCopy(std::string key, std::string value, size_t offset);
void resolve(std::vector<const VariableValue *> *l);
void resolve(std::vector<const VariableValue *> *l,
void resolve(std::vector<std::shared_ptr<const VariableValue>> *l);
void resolve(std::vector<std::shared_ptr<const VariableValue>> *l,
variables::KeyExclusions &ke);
void resolve(const std::string &key,
std::vector<const VariableValue *> *l);
std::vector<std::shared_ptr<const VariableValue>> *l);
void resolveRegularExpression(Utils::Regex *r,
std::vector<const VariableValue *> *l);
std::vector<std::shared_ptr<const VariableValue>> *l);
void resolveRegularExpression(Utils::Regex *r,
std::vector<const VariableValue *> *l,
std::vector<std::shared_ptr<const VariableValue>> *l,
variables::KeyExclusions &ke);
std::unique_ptr<std::string> resolveFirst(const std::string &key);

View File

@@ -59,8 +59,6 @@ class AnchoredVariable {
m_var(a.m_var) { }
*/
~AnchoredVariable();
void unset();
void set(const std::string &a, size_t offset);
void set(const bpstd::string_view &a, size_t offset);
@@ -71,7 +69,7 @@ class AnchoredVariable {
void append(const std::string &a, size_t offset,
bool spaceSeparator, int size);
void evaluate(std::vector<const VariableValue *> *l);
void evaluate(std::vector<std::shared_ptr<const VariableValue>> *l);
std::string * evaluate();
std::unique_ptr<std::string> resolveFirst();
@@ -81,7 +79,7 @@ class AnchoredVariable {
std::string m_value;
private:
VariableValue *m_var;
VariableValue m_var;
};
} // namespace modsecurity

View File

@@ -60,12 +60,12 @@ class Collection {
const std::string& var) = 0;
virtual void resolveSingleMatch(const std::string& var,
std::vector<const VariableValue *> *l) = 0;
std::vector<std::shared_ptr<const VariableValue>> *l) = 0;
virtual void resolveMultiMatches(const std::string& var,
std::vector<const VariableValue *> *l,
std::vector<std::shared_ptr<const VariableValue>> *l,
variables::KeyExclusions &ke) = 0;
virtual void resolveRegularExpression(const std::string& var,
std::vector<const VariableValue *> *l,
std::vector<std::shared_ptr<const VariableValue>> *l,
variables::KeyExclusions &ke) = 0;
@@ -146,7 +146,7 @@ class Collection {
/* resolveSingleMatch */
virtual void resolveSingleMatch(const std::string& var,
std::string compartment, std::vector<const VariableValue *> *l) {
std::string compartment, std::vector<std::shared_ptr<const VariableValue>> *l) {
std::string nkey = compartment + "::" + var;
resolveSingleMatch(nkey, l);
}
@@ -154,7 +154,7 @@ class Collection {
virtual void resolveSingleMatch(const std::string& var,
std::string compartment, std::string compartment2,
std::vector<const VariableValue *> *l) {
std::vector<std::shared_ptr<const VariableValue>> *l) {
std::string nkey = compartment + "::" + compartment2 + "::" + var;
resolveSingleMatch(nkey, l);
}
@@ -162,7 +162,7 @@ class Collection {
/* resolveMultiMatches */
virtual void resolveMultiMatches(const std::string& var,
std::string compartment, std::vector<const VariableValue *> *l,
std::string compartment, std::vector<std::shared_ptr<const VariableValue>> *l,
variables::KeyExclusions &ke) {
std::string nkey = compartment + "::" + var;
resolveMultiMatches(nkey, l, ke);
@@ -171,7 +171,7 @@ class Collection {
virtual void resolveMultiMatches(const std::string& var,
std::string compartment, std::string compartment2,
std::vector<const VariableValue *> *l,
std::vector<std::shared_ptr<const VariableValue>> *l,
variables::KeyExclusions &ke) {
std::string nkey = compartment + "::" + compartment2 + "::" + var;
resolveMultiMatches(nkey, l, ke);
@@ -180,7 +180,7 @@ class Collection {
/* resolveRegularExpression */
virtual void resolveRegularExpression(const std::string& var,
std::string compartment, std::vector<const VariableValue *> *l,
std::string compartment, std::vector<std::shared_ptr<const VariableValue>> *l,
variables::KeyExclusions &ke) {
std::string nkey = compartment + "::" + var;
resolveRegularExpression(nkey, l, ke);
@@ -189,7 +189,7 @@ class Collection {
virtual void resolveRegularExpression(const std::string& var,
std::string compartment, std::string compartment2,
std::vector<const VariableValue *> *l, variables::KeyExclusions &ke) {
std::vector<std::shared_ptr<const VariableValue>> *l, variables::KeyExclusions &ke) {
std::string nkey = compartment + "::" + compartment2 + "::" + var;
resolveRegularExpression(nkey, l, ke);
}

View File

@@ -37,7 +37,7 @@ class VariableOrigin {
: m_length(0),
m_offset(0) { }
std::string toText() {
std::string toText() const {
std::string offset = std::to_string(m_offset);
std::string len = std::to_string(m_length);
return "v" + offset + "," + len;

View File

@@ -37,7 +37,7 @@ namespace modsecurity {
class Collection;
class VariableValue {
public:
using Origins = std::list<std::unique_ptr<VariableOrigin>>;
using Origins = std::vector<VariableOrigin>;
explicit VariableValue(const std::string *key,
const std::string *value = nullptr)
@@ -63,10 +63,10 @@ class VariableValue {
m_value(o->m_value)
{
for (auto &i : o->m_orign) {
std::unique_ptr<VariableOrigin> origin(new VariableOrigin());
origin->m_offset = i->m_offset;
origin->m_length = i->m_length;
m_orign.push_back(std::move(origin));
VariableOrigin *origin(new VariableOrigin());
origin->m_offset = i.m_offset;
origin->m_length = i.m_length;
m_orign.push_back(*origin);
}
}
@@ -77,10 +77,10 @@ class VariableValue {
m_value(o.m_value)
{
for (auto &i : o.m_orign) {
std::unique_ptr<VariableOrigin> origin(new VariableOrigin());
origin->m_offset = i->m_offset;
origin->m_length = i->m_length;
m_orign.push_back(std::move(origin));
VariableOrigin *origin(new VariableOrigin());
origin->m_offset = i.m_offset;
origin->m_length = i.m_length;
m_orign.push_back(*origin);
}
}
@@ -109,7 +109,7 @@ class VariableValue {
}
void addOrigin(std::unique_ptr<VariableOrigin> origin) {
void addOrigin(VariableOrigin origin) {
m_orign.push_back(std::move(origin));
}