Skip to content

Commit 5933729

Browse files
Al2KlimovAl2Klimov
committed
Expression#GetReference(): ref, not copy, index if possible
to avoid malloc().
1 parent a65f2d6 commit 5933729

File tree

2 files changed

+70
-24
lines changed

2 files changed

+70
-24
lines changed

‎lib/config/expression.cpp‎

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,15 @@ using namespace icinga;
2222
boost::signals2::signal<void (ScriptFrame&, ScriptError *ex, const DebugInfo&)> Expression::OnBreakpoint;
2323
boost::thread_specific_ptr<bool> l_InBreakpointHandler;
2424

25+
RefIndex::RefIndex() : m_Foreign(&m_Own)
26+
{ }
27+
28+
void RefIndex::Set(String own)
29+
{
30+
m_Own = std::move(own);
31+
m_Foreign = &m_Own;
32+
}
33+
2534
Expression::~Expression()
2635
{ }
2736

@@ -63,7 +72,7 @@ ExpressionResult Expression::Evaluate(ScriptFrame& frame, DebugHint *dhint) cons
6372
}
6473
}
6574

66-
bool Expression::GetReference(ScriptFrame& frame, bool init_dict, Value *parent, String *index, DebugHint **dhint) const
75+
bool Expression::GetReference(ScriptFrame& frame, bool init_dict, Value *parent, RefIndex *index, DebugHint **dhint) const
6776
{
6877
return false;
6978
}
@@ -122,9 +131,9 @@ ExpressionResult VariableExpression::DoEvaluate(ScriptFrame& frame, DebugHint *d
122131
return ScriptGlobal::Get(m_Variable);
123132
}
124133

125-
bool VariableExpression::GetReference(ScriptFrame& frame, bool init_dict, Value *parent, String *index, DebugHint **dhint) const
134+
bool VariableExpression::GetReference(ScriptFrame& frame, bool init_dict, Value *parent, RefIndex *index, DebugHint **dhint) const
126135
{
127-
*index = m_Variable;
136+
index->Set(&m_Variable);
128137

129138
if (frame.Locals && frame.Locals->Contains(m_Variable)) {
130139
*parent = frame.Locals;
@@ -152,15 +161,15 @@ bool VariableExpression::GetReference(ScriptFrame& frame, bool init_dict, Value
152161
ExpressionResult RefExpression::DoEvaluate(ScriptFrame& frame, DebugHint *dhint) const
153162
{
154163
Value parent;
155-
String index;
164+
RefIndex index;
156165

157166
if (!m_Operand->GetReference(frame, false, &parent, &index, &dhint))
158167
BOOST_THROW_EXCEPTION(ScriptError("Cannot obtain reference for expression.", m_DebugInfo));
159168

160169
if (!parent.IsObject())
161170
BOOST_THROW_EXCEPTION(ScriptError("Cannot obtain reference for expression because parent is not an object.", m_DebugInfo));
162171

163-
return new Reference(parent, index);
172+
return new Reference(parent, index.Get());
164173
}
165174

166175
ExpressionResult DerefExpression::DoEvaluate(ScriptFrame& frame, DebugHint *dhint) const
@@ -177,7 +186,7 @@ ExpressionResult DerefExpression::DoEvaluate(ScriptFrame& frame, DebugHint *dhin
177186
return ref->Get();
178187
}
179188

180-
bool DerefExpression::GetReference(ScriptFrame& frame, bool init_dict, Value *parent, String *index, DebugHint **dhint) const
189+
bool DerefExpression::GetReference(ScriptFrame& frame, bool init_dict, Value *parent, RefIndex *index, DebugHint **dhint) const
181190
{
182191
ExpressionResult operand = m_Operand->Evaluate(frame);
183192
if (operand.GetCode() != ResultOK)
@@ -186,7 +195,7 @@ bool DerefExpression::GetReference(ScriptFrame& frame, bool init_dict, Value *pa
186195
Reference::Ptr ref = operand.GetValue();
187196

188197
*parent = ref->GetParent();
189-
*index = ref->GetIndex();
198+
index->Set(ref->GetIndex());
190199
return true;
191200
}
192201

@@ -449,10 +458,10 @@ ExpressionResult LogicalOrExpression::DoEvaluate(ScriptFrame& frame, DebugHint *
449458
ExpressionResult FunctionCallExpression::DoEvaluate(ScriptFrame& frame, DebugHint *dhint) const
450459
{
451460
Value self, vfunc;
452-
String index;
461+
RefIndex index;
453462

454463
if (m_FName->GetReference(frame, false, &self, &index))
455-
vfunc = VMOps::GetField(self, index, frame.Sandboxed, m_DebugInfo);
464+
vfunc = VMOps::GetField(self, index.Get(), frame.Sandboxed, m_DebugInfo);
456465
else {
457466
ExpressionResult vfuncres = m_FName->Evaluate(frame);
458467
CHECK_RESULT(vfuncres);
@@ -611,7 +620,7 @@ ExpressionResult SetExpression::DoEvaluate(ScriptFrame& frame, DebugHint *dhint)
611620
DebugHint *psdhint = dhint;
612621

613622
Value parent;
614-
String index;
623+
RefIndex index;
615624

616625
if (!m_Operand1->GetReference(frame, true, &parent, &index, &psdhint))
617626
BOOST_THROW_EXCEPTION(ScriptError("Expression cannot be assigned to.", m_DebugInfo));
@@ -620,7 +629,7 @@ ExpressionResult SetExpression::DoEvaluate(ScriptFrame& frame, DebugHint *dhint)
620629
CHECK_RESULT(operand2);
621630

622631
if (m_Op != OpSetLiteral) {
623-
Value object = VMOps::GetField(parent, index, frame.Sandboxed, m_DebugInfo);
632+
Value object = VMOps::GetField(parent, index.Get(), frame.Sandboxed, m_DebugInfo);
624633

625634
switch (m_Op) {
626635
case OpSetAdd:
@@ -652,7 +661,7 @@ ExpressionResult SetExpression::DoEvaluate(ScriptFrame& frame, DebugHint *dhint)
652661
}
653662
}
654663

655-
VMOps::SetField(parent, index, operand2.GetValue(), m_OverrideFrozen, m_DebugInfo);
664+
VMOps::SetField(parent, index.Get(), operand2.GetValue(), m_OverrideFrozen, m_DebugInfo);
656665

657666
if (psdhint) {
658667
psdhint->AddMessage("=", m_DebugInfo);
@@ -745,10 +754,10 @@ ExpressionResult IndexerExpression::DoEvaluate(ScriptFrame& frame, DebugHint *dh
745754
return VMOps::GetField(operand1.GetValue(), operand2.GetValue(), frame.Sandboxed, m_DebugInfo);
746755
}
747756

748-
bool IndexerExpression::GetReference(ScriptFrame& frame, bool init_dict, Value *parent, String *index, DebugHint **dhint) const
757+
bool IndexerExpression::GetReference(ScriptFrame& frame, bool init_dict, Value *parent, RefIndex *index, DebugHint **dhint) const
749758
{
750759
Value vparent;
751-
String vindex;
760+
RefIndex vindex;
752761
DebugHint *psdhint = nullptr;
753762
bool free_psd = false;
754763

@@ -765,29 +774,29 @@ bool IndexerExpression::GetReference(ScriptFrame& frame, bool init_dict, Value *
765774

766775
if (vparent.IsObject()) {
767776
Object::Ptr oparent = vparent;
768-
has_field = oparent->HasOwnField(vindex);
777+
has_field = oparent->HasOwnField(vindex.Get());
769778
}
770779

771780
if (has_field)
772-
old_value = VMOps::GetField(vparent, vindex, frame.Sandboxed, m_Operand1->GetDebugInfo());
781+
old_value = VMOps::GetField(vparent, vindex.Get(), frame.Sandboxed, m_Operand1->GetDebugInfo());
773782

774783
if (old_value.IsEmpty() && !old_value.IsString())
775-
VMOps::SetField(vparent, vindex, new Dictionary(), m_OverrideFrozen, m_Operand1->GetDebugInfo());
784+
VMOps::SetField(vparent, vindex.Get(), new Dictionary(), m_OverrideFrozen, m_Operand1->GetDebugInfo());
776785
}
777786

778-
*parent = VMOps::GetField(vparent, vindex, frame.Sandboxed, m_DebugInfo);
787+
*parent = VMOps::GetField(vparent, vindex.Get(), frame.Sandboxed, m_DebugInfo);
779788
free_psd = true;
780789
} else {
781790
ExpressionResult operand1 = m_Operand1->Evaluate(frame);
782791
*parent = operand1.GetValue();
783792
}
784793

785794
ExpressionResult operand2 = m_Operand2->Evaluate(frame);
786-
*index = operand2.GetValue();
795+
index->Set(operand2.GetValue());
787796

788797
if (dhint) {
789798
if (psdhint)
790-
*dhint = new DebugHint(psdhint->GetChild(*index));
799+
*dhint = new DebugHint(psdhint->GetChild(index->Get()));
791800
else
792801
*dhint = nullptr;
793802
}

‎lib/config/expression.hpp‎

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,37 @@ struct ExpressionResult
176176
if (res.GetCode() == ResultBreak) \
177177
break; \
178178

179+
/**
180+
* Abstracts a maybe owned String
181+
*
182+
* @ingroup config
183+
*/
184+
class RefIndex
185+
{
186+
public:
187+
RefIndex();
188+
RefIndex(const RefIndex&) = delete;
189+
RefIndex(RefIndex&&) = delete;
190+
RefIndex& operator=(const RefIndex&) = delete;
191+
RefIndex& operator=(RefIndex&&) = delete;
192+
193+
inline const String& Get() const noexcept
194+
{
195+
return *m_Foreign;
196+
}
197+
198+
inline void Set(const String* foreign) noexcept
199+
{
200+
m_Foreign = foreign;
201+
}
202+
203+
void Set(String own);
204+
205+
private:
206+
const String* m_Foreign;
207+
String m_Own;
208+
};
209+
179210
/**
180211
* @ingroup config
181212
*/
@@ -191,7 +222,13 @@ class Expression : public SharedObject
191222
Expression& operator=(const Expression&) = delete;
192223

193224
ExpressionResult Evaluate(ScriptFrame& frame, DebugHint *dhint = nullptr) const;
194-
virtual bool GetReference(ScriptFrame& frame, bool init_dict, Value *parent, String *index, DebugHint **dhint = nullptr) const;
225+
226+
/**
227+
* Caution! On return `*index` may point to a String somewhere in `this` (or subAST).
228+
* Keep `this` alive while using `*index`!
229+
*/
230+
virtual bool GetReference(ScriptFrame& frame, bool init_dict, Value *parent, RefIndex *index, DebugHint **dhint = nullptr) const;
231+
195232
virtual const DebugInfo& GetDebugInfo() const;
196233

197234
virtual ExpressionResult DoEvaluate(ScriptFrame& frame, DebugHint *dhint) const = 0;
@@ -310,7 +347,7 @@ class VariableExpression final : public DebuggableExpression
310347

311348
protected:
312349
ExpressionResult DoEvaluate(ScriptFrame& frame, DebugHint *dhint) const override;
313-
bool GetReference(ScriptFrame& frame, bool init_dict, Value *parent, String *index, DebugHint **dhint) const override;
350+
bool GetReference(ScriptFrame& frame, bool init_dict, Value *parent, RefIndex *index, DebugHint **dhint) const override;
314351

315352
private:
316353
String m_Variable;
@@ -328,7 +365,7 @@ class DerefExpression final : public UnaryExpression
328365

329366
protected:
330367
ExpressionResult DoEvaluate(ScriptFrame& frame, DebugHint *dhint) const override;
331-
bool GetReference(ScriptFrame& frame, bool init_dict, Value *parent, String *index, DebugHint **dhint) const override;
368+
bool GetReference(ScriptFrame& frame, bool init_dict, Value *parent, RefIndex *index, DebugHint **dhint) const override;
332369
};
333370

334371
class RefExpression final : public UnaryExpression
@@ -761,7 +798,7 @@ class IndexerExpression final : public BinaryExpression
761798
bool m_OverrideFrozen{false};
762799

763800
ExpressionResult DoEvaluate(ScriptFrame& frame, DebugHint *dhint) const override;
764-
bool GetReference(ScriptFrame& frame, bool init_dict, Value *parent, String *index, DebugHint **dhint) const override;
801+
bool GetReference(ScriptFrame& frame, bool init_dict, Value *parent, RefIndex *index, DebugHint **dhint) const override;
765802

766803
friend void BindToScope(std::unique_ptr<Expression>& expr, ScopeSpecifier scopeSpec);
767804
};

0 commit comments

Comments
 (0)