From be83ee5a22d4a66adfaf21720e66c9b22054f0f2 Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Sun, 16 Apr 2023 21:07:43 -0400 Subject: [PATCH 1/4] Pull handle out of loop in Runtime::collectAttributes --- runtime/runtime.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/runtime/runtime.cpp b/runtime/runtime.cpp index 44f92290a..8c708d3e3 100644 --- a/runtime/runtime.cpp +++ b/runtime/runtime.cpp @@ -2877,6 +2877,7 @@ void Runtime::collectAttributes(const Code& code, const Dict& attributes) { HandleScope scope(thread); Bytes bc(&scope, code.code()); Tuple names(&scope, code.names()); + Str name(&scope, Str::empty()); word len = bc.length(); for (word i = 0; i < len - 3; i += 2) { @@ -2895,7 +2896,7 @@ void Runtime::collectAttributes(const Code& code, const Dict& attributes) { continue; } word name_index = bc.byteAt(i + 3); - Str name(&scope, names.at(name_index)); + name = names.at(name_index); dictAtPutByStr(thread, attributes, name, name); } } From 5737ef16c18268dfdcfe8124da754fed6ad22679 Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Sun, 16 Apr 2023 21:50:46 -0400 Subject: [PATCH 2/4] Use Set instead of Dict for Runtime::collectAttributes The dict is wasted space because we don't need two entries. --- runtime/runtime-test.cpp | 24 ++++++++++-------------- runtime/runtime.cpp | 5 +++-- runtime/runtime.h | 2 +- runtime/type-builtins.cpp | 2 +- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/runtime/runtime-test.cpp b/runtime/runtime-test.cpp index 73249b579..c3cffee49 100644 --- a/runtime/runtime-test.cpp +++ b/runtime/runtime-test.cpp @@ -1403,16 +1403,15 @@ TEST_F(RuntimeTest, CollectAttributes) { Code code0(&scope, newCodeWithBytesConstsNamesLocals(bytecode0, consts, names, &locals)); - Dict attributes(&scope, runtime_->newDict()); + Set attributes(&scope, runtime_->newSet()); runtime_->collectAttributes(code0, attributes); // We should have collected a single attribute: 'foo' EXPECT_EQ(attributes.numItems(), 1); // Check that we collected 'foo' - Object result(&scope, dictAtByStr(thread_, attributes, foo)); - ASSERT_TRUE(result.isStr()); - EXPECT_TRUE(Str::cast(*result).equals(*foo)); + word hash = strHash(thread_, *foo); + EXPECT_TRUE(setIncludes(thread_, attributes, foo, hash)); // Bytecode for the snippet: // @@ -1430,14 +1429,12 @@ TEST_F(RuntimeTest, CollectAttributes) { EXPECT_EQ(attributes.numItems(), 3); // Check that we collected 'bar' - result = dictAtByStr(thread_, attributes, bar); - ASSERT_TRUE(result.isStr()); - EXPECT_TRUE(Str::cast(*result).equals(*bar)); + hash = strHash(thread_, *bar); + EXPECT_TRUE(setIncludes(thread_, attributes, bar, hash)); // Check that we collected 'baz' - result = dictAtByStr(thread_, attributes, baz); - ASSERT_TRUE(result.isStr()); - EXPECT_TRUE(Str::cast(*result).equals(*baz)); + hash = strHash(thread_, *baz); + EXPECT_TRUE(setIncludes(thread_, attributes, baz, hash)); } TEST_F(RuntimeTest, CollectAttributesWithExtendedArg) { @@ -1467,16 +1464,15 @@ TEST_F(RuntimeTest, CollectAttributesWithExtendedArg) { Code code(&scope, newCodeWithBytesConstsNamesLocals(bytecode, consts, names, &locals)); - Dict attributes(&scope, runtime_->newDict()); + Set attributes(&scope, runtime_->newSet()); runtime_->collectAttributes(code, attributes); // We should have collected a single attribute: 'foo' EXPECT_EQ(attributes.numItems(), 1); // Check that we collected 'foo' - Object result(&scope, dictAtByStr(thread_, attributes, foo)); - ASSERT_TRUE(result.isStr()); - EXPECT_TRUE(Str::cast(*result).equals(*foo)); + word hash = strHash(thread_, *foo); + EXPECT_TRUE(setIncludes(thread_, attributes, foo, hash)); } TEST_F(RuntimeTest, GetTypeConstructor) { diff --git a/runtime/runtime.cpp b/runtime/runtime.cpp index 8c708d3e3..d3571235a 100644 --- a/runtime/runtime.cpp +++ b/runtime/runtime.cpp @@ -2872,7 +2872,7 @@ RawObject Runtime::newWeakRef(Thread* thread, const Object& referent) { return *ref; } -void Runtime::collectAttributes(const Code& code, const Dict& attributes) { +void Runtime::collectAttributes(const Code& code, const Set& attributes) { Thread* thread = Thread::current(); HandleScope scope(thread); Bytes bc(&scope, code.code()); @@ -2897,7 +2897,8 @@ void Runtime::collectAttributes(const Code& code, const Dict& attributes) { } word name_index = bc.byteAt(i + 3); name = names.at(name_index); - dictAtPutByStr(thread, attributes, name, name); + word hash = strHash(thread, *name); + setAdd(thread, attributes, name, hash); } } diff --git a/runtime/runtime.h b/runtime/runtime.h index e3450618d..43e4cfacd 100644 --- a/runtime/runtime.h +++ b/runtime/runtime.h @@ -561,7 +561,7 @@ class Runtime { // Performs a simple scan of the bytecode and collects all attributes that // are set via `self. =` into attributes. - void collectAttributes(const Code& code, const Dict& attributes); + void collectAttributes(const Code& code, const Set& attributes); // Returns type's __init__ method, or None RawObject classConstructor(const Type& type); diff --git a/runtime/type-builtins.cpp b/runtime/type-builtins.cpp index a5f932f90..0c78b2430 100644 --- a/runtime/type-builtins.cpp +++ b/runtime/type-builtins.cpp @@ -667,7 +667,7 @@ static word estimateNumAttributes(Thread* thread, const Type& type) { // each class in the MRO. This is used to determine the number of slots // allocated for in-object attributes when instances are created. Tuple mro(&scope, type.mro()); - Dict attr_names(&scope, runtime->newDict()); + Set attr_names(&scope, runtime->newSet()); for (word i = 0; i < mro.length(); i++) { Type mro_type(&scope, mro.at(i)); Object maybe_init(&scope, runtime->classConstructor(mro_type)); From a82b34e3538f43d329d421853ea3c469ba43dbaa Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Sun, 16 Apr 2023 21:57:00 -0400 Subject: [PATCH 3/4] Use kCompilerCodeUnitSize instead of magic 2 in Runtime::collectAttributes --- runtime/runtime.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/runtime.cpp b/runtime/runtime.cpp index d3571235a..0f5fc715e 100644 --- a/runtime/runtime.cpp +++ b/runtime/runtime.cpp @@ -2880,11 +2880,11 @@ void Runtime::collectAttributes(const Code& code, const Set& attributes) { Str name(&scope, Str::empty()); word len = bc.length(); - for (word i = 0; i < len - 3; i += 2) { + for (word i = 0; i < len - 3; i += kCompilerCodeUnitSize) { // If the current instruction is EXTENDED_ARG we must skip it and the next // instruction. if (bc.byteAt(i) == Bytecode::EXTENDED_ARG) { - i += 2; + i += kCompilerCodeUnitSize; continue; } // Check for LOAD_FAST 0 (self) From f7576f0ec167f1e4ae24771986872c7b2df6ac5b Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Sun, 16 Apr 2023 22:08:42 -0400 Subject: [PATCH 4/4] Skip all EXTENDED_ARG because opcodes can have more than one --- runtime/runtime.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/runtime.cpp b/runtime/runtime.cpp index 0f5fc715e..15b06199d 100644 --- a/runtime/runtime.cpp +++ b/runtime/runtime.cpp @@ -2883,7 +2883,7 @@ void Runtime::collectAttributes(const Code& code, const Set& attributes) { for (word i = 0; i < len - 3; i += kCompilerCodeUnitSize) { // If the current instruction is EXTENDED_ARG we must skip it and the next // instruction. - if (bc.byteAt(i) == Bytecode::EXTENDED_ARG) { + while (bc.byteAt(i) == Bytecode::EXTENDED_ARG) { i += kCompilerCodeUnitSize; continue; }