Skip to content

Commit 74a4986

Browse files
undingenundingen
committed
cleanup even more
1 parent 8fb4cb5 commit 74a4986

File tree

3 files changed

+49
-57
lines changed

3 files changed

+49
-57
lines changed

‎src/core/cfg.cpp

Lines changed: 29 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -688,11 +688,13 @@ class CFGVisitor : public ASTVisitor {
688688
return createDstName(rtn);
689689
}
690690

691-
int addConst(Box* o) {
691+
int addConst(STOLEN(Box*) o) {
692692
// make sure all consts are unique
693693
auto it = consts.find(o);
694-
if (it != consts.end())
694+
if (it != consts.end()) {
695+
Py_DECREF(o);
695696
return it->second;
697+
}
696698
int vreg = code_constants.createVRegEntryForConstant(o);
697699
consts[o] = vreg;
698700
return vreg;
@@ -707,20 +709,17 @@ class CFGVisitor : public ASTVisitor {
707709

708710
TmpValue remapNum(AST_Num* num) {
709711
Box* o = createConstObject(num);
710-
AUTO_DECREF(o);
711712
return TmpValue(addConst(o), num->lineno);
712713
}
713714

714715
TmpValue makeStr(llvm::StringRef str, int lineno = 0) {
715716
AST_Str ast_str(str.str());
716717
Box* o = createConstObject(&ast_str);
717-
AUTO_DECREF(o);
718718
return TmpValue(addConst(o), lineno);
719719
}
720720

721721
TmpValue remapStr(AST_Str* str) {
722722
Box* o = createConstObject(str);
723-
AUTO_DECREF(o);
724723
return TmpValue(addConst(o), str->lineno);
725724
}
726725

@@ -729,12 +728,11 @@ class CFGVisitor : public ASTVisitor {
729728
ast_num.num_type = AST_Num::INT;
730729
ast_num.n_int = n;
731730
Box* o = createConstObject(&ast_num);
732-
AUTO_DECREF(o);
733731
return TmpValue(addConst(o), lineno);
734732
}
735733

736734
TmpValue makeNone(int lineno) {
737-
int vreg_const = addConst(Py_None);
735+
int vreg_const = addConst(incref(Py_None));
738736
return TmpValue(vreg_const, lineno);
739737
}
740738

@@ -1250,7 +1248,6 @@ class CFGVisitor : public ASTVisitor {
12501248
for (int i = 0; i < node->keywords.size(); ++i) {
12511249
tuple->elts[i] = incref(node->keywords[i]->arg.getBox());
12521250
}
1253-
code_constants.addOwnedRef(tuple);
12541251
rtn_shared->index_keyword_names = addConst(tuple);
12551252
}
12561253

@@ -1360,7 +1357,7 @@ class CFGVisitor : public ASTVisitor {
13601357
}
13611358

13621359
TmpValue remapEllipsis(AST_Ellipsis* node) {
1363-
int vreg_const = addConst(Ellipsis);
1360+
int vreg_const = addConst(incref(Ellipsis));
13641361
return TmpValue(vreg_const, node->lineno);
13651362
}
13661363

@@ -1438,8 +1435,7 @@ class CFGVisitor : public ASTVisitor {
14381435
BoxedCode* code = cfgizer->runRecursively(new_body, gen_name, node->lineno, genexp_args, node);
14391436
BST_MakeFunction* mkfunc = allocAndPush<BST_MakeFunction>(0, 0);
14401437
mkfunc->lineno = node->lineno;
1441-
mkfunc->vreg_code_obj = code_constants.createVRegEntryForConstant(code);
1442-
code_constants.addOwnedRef(code);
1438+
mkfunc->vreg_code_obj = addConst(code);
14431439
TmpValue func_var_name = createDstName(mkfunc);
14441440

14451441
return makeCall(func_var_name, { first });
@@ -1498,8 +1494,7 @@ class CFGVisitor : public ASTVisitor {
14981494
BoxedCode* code = cfgizer->runRecursively(new_body, comp_name, node->lineno, args, node);
14991495
BST_MakeFunction* mkfunc = allocAndPush<BST_MakeFunction>(0, 0);
15001496
mkfunc->lineno = node->lineno;
1501-
mkfunc->vreg_code_obj = code_constants.createVRegEntryForConstant(code);
1502-
code_constants.addOwnedRef(code);
1497+
mkfunc->vreg_code_obj = addConst(code);
15031498
TmpValue func_var_name = createDstName(mkfunc);
15041499

15051500
return makeCall(func_var_name, { first });
@@ -1554,8 +1549,7 @@ class CFGVisitor : public ASTVisitor {
15541549

15551550
auto name = getStaticString("<lambda>");
15561551
auto* code = cfgizer->runRecursively({ stmt }, name, mkfn->lineno, node->args, node);
1557-
mkfn->vreg_code_obj = code_constants.createVRegEntryForConstant(code);
1558-
code_constants.addOwnedRef(code);
1552+
mkfn->vreg_code_obj = addConst(code);
15591553

15601554
return createDstName(mkfn);
15611555
}
@@ -1666,18 +1660,18 @@ class CFGVisitor : public ASTVisitor {
16661660
if (str->str_type == AST_Str::STR) {
16671661
BoxedString*& o = str_constants[str->str_data];
16681662
// we always intern the string
1669-
if (!o) {
1663+
if (!o)
16701664
o = internStringMortal(str->str_data);
1671-
code_constants.addOwnedRef(o);
1672-
}
1673-
return incref(o);
1665+
else
1666+
incref(o);
1667+
return o;
16741668
} else if (str->str_type == AST_Str::UNICODE) {
16751669
Box*& r = unicode_constants[str->str_data];
1676-
if (!r) {
1670+
if (!r)
16771671
r = decodeUTF8StringPtr(str->str_data);
1678-
code_constants.addOwnedRef(r);
1679-
}
1680-
return incref(r);
1672+
else
1673+
incref(r);
1674+
return r;
16811675
} else
16821676
RELEASE_ASSERT(0, "not implemented");
16831677
} else if (node->type == AST_TYPE::Num) {
@@ -1688,18 +1682,18 @@ class CFGVisitor : public ASTVisitor {
16881682
return incref(code_constants.getFloatConstant(num->n_float));
16891683
else if (num->num_type == AST_Num::LONG) {
16901684
Box*& r = long_constants[num->n_long];
1691-
if (!r) {
1685+
if (!r)
16921686
r = createLong(num->n_long);
1693-
code_constants.addOwnedRef(r);
1694-
}
1695-
return incref(r);
1687+
else
1688+
incref(r);
1689+
return r;
16961690
} else if (num->num_type == AST_Num::COMPLEX) {
16971691
Box*& r = imaginary_constants[getDoubleBits(num->n_float)];
1698-
if (!r) {
1692+
if (!r)
16991693
r = createPureImaginary(num->n_float);
1700-
code_constants.addOwnedRef(r);
1701-
}
1702-
return incref(r);
1694+
else
1695+
incref(r);
1696+
return r;
17031697
} else
17041698
RELEASE_ASSERT(0, "not implemented");
17051699
} else if (node->type == AST_TYPE::Tuple) {
@@ -1720,8 +1714,7 @@ class CFGVisitor : public ASTVisitor {
17201714

17211715
// handle tuples where all elements are constants as a constant
17221716
if (isConstObject(node)) {
1723-
BoxedTuple* tuple = (BoxedTuple*)createConstObject(node);
1724-
code_constants.addOwnedRef(tuple);
1717+
Box* tuple = createConstObject(node);
17251718
return TmpValue(addConst(tuple), node->lineno);
17261719
}
17271720

@@ -1968,8 +1961,7 @@ class CFGVisitor : public ASTVisitor {
19681961
unmapExpr(bases_name, &mkclass->vreg_bases_tuple);
19691962

19701963
auto* code = cfgizer->runRecursively(node->body, node->name.getBox(), node->lineno, NULL, node);
1971-
mkclass->vreg_code_obj = code_constants.createVRegEntryForConstant(code);
1972-
code_constants.addOwnedRef(code);
1964+
mkclass->vreg_code_obj = addConst(code);
19731965

19741966
auto tmp = createDstName(mkclass);
19751967
pushAssign(TmpValue(scoping->mangleName(node->name), node->lineno), tmp);
@@ -2003,8 +1995,7 @@ class CFGVisitor : public ASTVisitor {
20031995
}
20041996

20051997
auto* code = cfgizer->runRecursively(node->body, node->name.getBox(), node->lineno, node->args, node);
2006-
mkfunc->vreg_code_obj = code_constants.createVRegEntryForConstant(code);
2007-
code_constants.addOwnedRef(code);
1998+
mkfunc->vreg_code_obj = addConst(code);
20081999
auto tmp = createDstName(mkfunc);
20092000
pushAssign(TmpValue(scoping->mangleName(node->name), node->lineno), tmp);
20102001

@@ -2097,7 +2088,6 @@ class CFGVisitor : public ASTVisitor {
20972088
for (int i = 0; i < node->names.size(); i++) {
20982089
tuple->elts[i] = internStringMortal(node->names[i]->name.s());
20992090
}
2100-
code_constants.addOwnedRef(tuple);
21012091
import->vreg_from = addConst(tuple);
21022092
unmapExpr(makeStr(node->module.s()), &import->vreg_name);
21032093

@@ -3547,6 +3537,7 @@ static std::pair<CFG*, CodeConstants> computeCFG(llvm::ArrayRef<AST_stmt*> body,
35473537

35483538
pruneUnnecessaryBlocks(rtn);
35493539

3540+
visitor.code_constants.optimizeSize();
35503541
rtn->bytecode.optimizeSize();
35513542
rtn->blocks.shrink_to_fit();
35523543

‎src/runtime/types.cpp

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4034,10 +4034,8 @@ int BoxedClosure::clear(Box* _o) noexcept {
40344034

40354035
BORROWED(BoxedInt*) CodeConstants::getIntConstant(int64_t n) const {
40364036
BoxedInt*& r = int_constants[n];
4037-
if (!r) {
4037+
if (!r)
40384038
r = (BoxedInt*)boxInt(n);
4039-
addOwnedRef(r);
4040-
}
40414039
return r;
40424040
}
40434041

@@ -4047,16 +4045,20 @@ BORROWED(BoxedFloat*) CodeConstants::getFloatConstant(double d) const {
40474045
memcpy(&double_as_int64, &d, sizeof(d));
40484046

40494047
BoxedFloat*& r = float_constants[double_as_int64];
4050-
if (!r) {
4048+
if (!r)
40514049
r = (BoxedFloat*)boxFloat(d);
4052-
addOwnedRef(r);
4053-
}
40544050
return r;
40554051
}
40564052

4057-
void CodeConstants::dealloc() const {
4058-
decrefArray(owned_refs.data(), owned_refs.size());
4059-
owned_refs.clear();
4053+
CodeConstants::~CodeConstants() {
4054+
if (!constants.empty())
4055+
decrefArray(constants.data(), constants.size());
4056+
for (auto&& e : int_constants) {
4057+
Py_DECREF(e.second);
4058+
}
4059+
for (auto&& e : float_constants) {
4060+
Py_DECREF(e.second);
4061+
}
40604062
}
40614063

40624064
#ifndef Py_REF_DEBUG

‎src/runtime/types.h

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,43 +1074,42 @@ class CodeConstants {
10741074
private:
10751075
// stores all constants accessible by vregs in the corrext order
10761076
// constants[-(vreg + 1)] will allow one to retrieve the constant for a vreg
1077+
// all entries are owned by code constants and will get decrefed in the destructor.
10771078
std::vector<Box*> constants;
10781079

1079-
// all objects we need to decref when the code object dies
1080-
mutable std::vector<Box*> owned_refs;
1081-
10821080
// Note: DenseMap doesn't work here since we don't prevent the tombstone/empty
10831081
// keys from reaching it.
1082+
// all entries are owned by code constants and will get decrefed in the destructor.
10841083
mutable std::unordered_map<int64_t, BoxedInt*> int_constants;
10851084
// I'm not sure how well it works to use doubles as hashtable keys; thankfully
10861085
// it's not a big deal if we get misses.
1086+
// all entries are owned by code constants and will get decrefed in the destructor.
10871087
mutable std::unordered_map<int64_t, BoxedFloat*> float_constants;
10881088

10891089
public:
10901090
CodeConstants() {}
10911091
CodeConstants(CodeConstants&&) = default;
10921092
CodeConstants& operator=(CodeConstants&&) = default;
1093-
~CodeConstants() { dealloc(); }
1093+
~CodeConstants();
10941094

10951095
BORROWED(Box*) getConstant(int vreg) const { return constants[-(vreg + 1)]; }
10961096

10971097
InternedString getInternedString(int vreg) const { return InternedString::unsafe((BoxedString*)getConstant(vreg)); }
10981098

10991099
// returns the vreg num for the constant (which is a negative number)
1100-
int createVRegEntryForConstant(Box* o) {
1100+
int createVRegEntryForConstant(STOLEN(Box*) o) {
11011101
constants.push_back(o);
11021102
return -constants.size();
11031103
}
11041104

1105-
1106-
void addOwnedRef(Box* o) const { owned_refs.emplace_back(o); }
1105+
void optimizeSize() {
1106+
constants.shrink_to_fit();
1107+
}
11071108

11081109
BORROWED(BoxedInt*) getIntConstant(int64_t n) const;
11091110
BORROWED(BoxedFloat*) getFloatConstant(double d) const;
11101111

1111-
int addInternedString(InternedString s) { return createVRegEntryForConstant(s.getBox()); }
1112-
1113-
void dealloc() const;
1112+
int addInternedString(InternedString s) { return createVRegEntryForConstant(incref(s.getBox())); }
11141113
};
11151114

11161115

0 commit comments

Comments
 (0)