Move method symbols into a new Method class (sorbet#4974)

Move method symbols into a new Method class. * Speeds up Sorbet typechecking on large projects by reducing the cost of defining/allocating method symbols. * Slightly reduces memory consumption due to smaller method symbol objects. * Reduces `SymbolRef` casts due to more specifically-typed APIs (e.g., `Method::owner` is a `ClassOrModuleRef`, `Method::typeArguments` is a `vector` of `TypeArgumentRef`, and `Symbol::typeMembers()` is now a `vector` of `TypeMemberRef`). * Removes some `SymbolRef` methods that are no longer required.
dougedey-shopify · Dec 9, 2021 · 480e422 · 480e422
1 parent 4e19022
commit 480e422
Show file tree

Hide file tree

Showing 48 changed files with 830 additions and 766 deletions.
diff --git a/ast/Trees.cc b/ast/Trees.cc
@@ -525,7 +525,7 @@ string MethodDef::toStringWithTabs(const core::GlobalState &gs, int tabs) const
             fmt::format_to(std::back_inserter(buf), "{}", a.toStringWithTabs(gs, tabs + 1));
         }
     } else {
-        for (auto &a : data->arguments()) {
+        for (auto &a : data->arguments) {
             if (!first) {
                 fmt::format_to(std::back_inserter(buf), ", ");
             }

diff --git a/cfg/Instructions.cc b/cfg/Instructions.cc
@@ -218,7 +218,7 @@ string LoadArg::showRaw(const core::GlobalState &gs, const CFG &cfg, int tabs) c
 }
 
 const core::ArgInfo &LoadArg::argument(const core::GlobalState &gs) const {
-    return this->method.data(gs)->arguments()[this->argId];
+    return this->method.data(gs)->arguments[this->argId];
 }
 
 string ArgPresent::toString(const core::GlobalState &gs, const CFG &cfg) const {
@@ -230,7 +230,7 @@ string ArgPresent::showRaw(const core::GlobalState &gs, const CFG &cfg, int tabs
 }
 
 const core::ArgInfo &ArgPresent::argument(const core::GlobalState &gs) const {
-    return this->method.data(gs)->arguments()[this->argId];
+    return this->method.data(gs)->arguments[this->argId];
 }
 
 string LoadYieldParams::toString(const core::GlobalState &gs, const CFG &cfg) const {

diff --git a/cfg/builder/builder_entry.cc b/cfg/builder/builder_entry.cc
@@ -10,7 +10,7 @@ namespace sorbet::cfg {
 unique_ptr<CFG> CFGBuilder::buildFor(core::Context ctx, ast::MethodDef &md) {
     Timer timeit(ctx.state.tracer(), "cfg");
     ENFORCE(md.symbol.exists());
-    ENFORCE(!md.symbol.data(ctx)->isOverloaded());
+    ENFORCE(!md.symbol.data(ctx)->flags.isOverloaded);
     unique_ptr<CFG> res(new CFG); // private constructor
     res->file = ctx.file;
     res->symbol = md.symbol.data(ctx)->dealiasMethod(ctx);
@@ -27,7 +27,7 @@ unique_ptr<CFG> CFGBuilder::buildFor(core::Context ctx, ast::MethodDef &md) {
         CFG::UnfreezeCFGLocalVariables unfreezeVars(*res);
         retSym = cctx.newTemporary(core::Names::returnMethodTemp());
 
-        auto selfClaz = md.symbol.data(ctx)->rebind();
+        auto selfClaz = md.symbol.data(ctx)->rebind;
         if (!selfClaz.exists()) {
             selfClaz = md.symbol.enclosingClass(ctx);
         }
@@ -38,8 +38,8 @@ unique_ptr<CFG> CFGBuilder::buildFor(core::Context ctx, ast::MethodDef &md) {
         BasicBlock *presentCont = entry;
         BasicBlock *defaultCont = nullptr;
 
-        auto &argInfos = md.symbol.data(ctx)->arguments();
-        bool isAbstract = md.symbol.data(ctx)->isAbstract();
+        auto &argInfos = md.symbol.data(ctx)->arguments;
+        bool isAbstract = md.symbol.data(ctx)->flags.isAbstract;
         bool seenKeyword = false;
         int i = -1;
         for (auto &argExpr : md.args) {

diff --git a/class_flatten/class_flatten.cc b/class_flatten/class_flatten.cc
@@ -96,9 +96,9 @@ class ClassFlattenWalk {
             // available to the containing static-init
             replacement = ast::MK::DefineTopClassOrModule(classDef->declLoc, classDef->symbol);
         }
-        ENFORCE(!sym.data(ctx)->arguments().empty(), "<static-init> method should already have a block arg symbol: {}",
+        ENFORCE(!sym.data(ctx)->arguments.empty(), "<static-init> method should already have a block arg symbol: {}",
                 sym.show(ctx));
-        ENFORCE(sym.data(ctx)->arguments().back().flags.isBlock, "Last argument symbol is not a block arg: {}",
+        ENFORCE(sym.data(ctx)->arguments.back().flags.isBlock, "Last argument symbol is not a block arg: {}",
                 sym.show(ctx));
 
         // Synthesize a block argument for this <static-init> block. This is rather fiddly,

diff --git a/compiler/IREmitter/IREmitter.cc b/compiler/IREmitter/IREmitter.cc
@@ -41,7 +41,7 @@ vector<core::ArgInfo::ArgFlags> getArgFlagsForBlockId(CompilerState &cs, int blo
     }
 
     vector<core::ArgInfo::ArgFlags> res;
-    for (auto &argInfo : method.data(cs)->arguments()) {
+    for (auto &argInfo : method.data(cs)->arguments) {
         res.emplace_back(argInfo.flags);
     }
 
@@ -637,7 +637,7 @@ llvm::BasicBlock *resolveJumpTarget(cfg::CFG &cfg, const IREmitterContext &irctx
 void emitUserBody(CompilerState &base, cfg::CFG &cfg, const IREmitterContext &irctx) {
     llvm::IRBuilder<> builder(base);
     auto startLoc = cfg.symbol.data(base)->loc();
-    auto &arguments = cfg.symbol.data(base)->arguments();
+    auto &arguments = cfg.symbol.data(base)->arguments;
     for (auto it = cfg.forwardsTopoSort.rbegin(); it != cfg.forwardsTopoSort.rend(); ++it) {
         cfg::BasicBlock *bb = *it;
         auto cs = base.withFunctionEntry(irctx.functionInitializersByFunction[bb->rubyRegionId]);
@@ -957,7 +957,7 @@ void IREmitter::run(CompilerState &cs, cfg::CFG &cfg, const ast::MethodDef &md)
     emitBlockExits(cs, cfg, irctx);
     emitPostProcess(cs, cfg, irctx);
 
-    if (md.symbol.data(cs)->isFinalMethod()) {
+    if (md.symbol.data(cs)->flags.isFinal) {
         emitDirectWrapper(cs, md, irctx);
     }
 

diff --git a/compiler/IREmitter/IREmitterContext.cc b/compiler/IREmitter/IREmitterContext.cc
@@ -380,7 +380,7 @@ CapturedVariables findCaptures(CompilerState &cs, const ast::MethodDef &mdef, cf
     TrackCaptures usage(aliases, blockLevels);
 
     int argId = -1;
-    auto &methodArguments = cfg.symbol.data(cs)->arguments();
+    auto &methodArguments = cfg.symbol.data(cs)->arguments;
     for (auto &arg : mdef.args) {
         argId += 1;
         ast::Local const *local = nullptr;
@@ -392,7 +392,7 @@ CapturedVariables findCaptures(CompilerState &cs, const ast::MethodDef &mdef, cf
         ENFORCE(local);
         auto localRef = cfg.enterLocal(local->localVariable);
         auto &argInfo = methodArguments[argId];
-        if (cfg.symbol.data(cs)->arguments()[argId].flags.isBlock) {
+        if (cfg.symbol.data(cs)->arguments[argId].flags.isBlock) {
             usage.blkArg = localRef;
         }
         usage.trackBlockArgument(cfg.entry(), localRef, argInfo.type);
@@ -499,9 +499,9 @@ llvm::DISubprogram *getDebugScope(CompilerState &cs, cfg::CFG &cfg, llvm::DIScop
     auto loc = cfg.symbol.data(cs)->loc();
 
     auto owner = cfg.symbol.data(cs)->owner;
-    std::string diName(owner.name(cs).shortName(cs));
+    std::string diName(owner.data(cs)->name.shortName(cs));
 
-    if (owner.isSingletonClass(cs)) {
+    if (owner.data(cs)->isSingletonClass(cs)) {
         diName += ".";
     } else {
         diName += "#";
@@ -1032,7 +1032,7 @@ IREmitterContext IREmitterContext::getSorbetBlocks2LLVMBlockMapping(CompilerStat
 
     // The method arguments are initialized here, while the block arguments are initialized when the blockCall header is
     // encountered in the loop below.
-    int numArgs = md.symbol.data(cs)->arguments().size();
+    int numArgs = md.symbol.data(cs)->arguments.size();
     argPresentVariables[0].resize(numArgs, cfg::LocalRef::noVariable());
 
     for (auto &b : cfg.basicBlocks) {

diff --git a/compiler/IREmitter/IREmitterHelpers.cc b/compiler/IREmitter/IREmitterHelpers.cc
@@ -43,7 +43,7 @@ string getFunctionNamePrefix(CompilerState &cs, core::ClassOrModuleRef sym) {
 } // namespace
 
 string IREmitterHelpers::getFunctionName(CompilerState &cs, core::MethodRef sym) {
-    auto owner = sym.data(cs)->owner.asClassOrModuleRef();
+    auto owner = sym.data(cs)->owner;
     auto maybeAttachedOwner = owner.data(cs)->attachedClass(cs);
     string prefix = "func_";
     if (maybeAttachedOwner.exists()) {
@@ -228,7 +228,7 @@ llvm::Value *IREmitterHelpers::maybeCheckReturnValue(CompilerState &cs, cfg::CFG
 
     // sorbet-runtime doesn't check this type for abstract methods, so we won't either.
     // TODO(froydnj): we should check this type.
-    if (!cfg.symbol.data(cs)->isAbstract()) {
+    if (!cfg.symbol.data(cs)->flags.isAbstract) {
         IREmitterHelpers::emitTypeTest(cs, builder, returnValue, expectedType, "Return value");
     }
 
@@ -364,7 +364,7 @@ bool IREmitterHelpers::hasBlockArgument(CompilerState &cs, int blockId, core::Me
         return blockLink->argFlags.back().isBlock;
     }
 
-    auto &args = method.data(cs)->arguments();
+    auto &args = method.data(cs)->arguments;
     if (args.empty()) {
         return false;
     }
@@ -447,7 +447,7 @@ IREmitterHelpers::isFinalMethod(const core::GlobalState &gs, core::TypePtr recvT
         return std::nullopt;
     }
 
-    if (!funSym.data(gs)->isFinalMethod()) {
+    if (!funSym.data(gs)->flags.isFinal) {
         return std::nullopt;
     }
 

diff --git a/compiler/IREmitter/Payload.cc b/compiler/IREmitter/Payload.cc
@@ -945,7 +945,7 @@ llvm::Value *Payload::buildInstanceVariableCache(CompilerState &cs, std::string_
 
 llvm::Value *Payload::getClassVariableStoreClass(CompilerState &cs, llvm::IRBuilderBase &builder,
                                                  const IREmitterContext &irctx) {
-    auto sym = irctx.cfg.symbol.data(cs)->owner.asClassOrModuleRef();
+    auto sym = irctx.cfg.symbol.data(cs)->owner;
     return Payload::getRubyConstant(cs, sym.data(cs)->topAttachedClass(cs), builder);
 }
 

diff --git a/compiler/IREmitter/SymbolBasedIntrinsics.cc b/compiler/IREmitter/SymbolBasedIntrinsics.cc
@@ -72,11 +72,10 @@ class CMethod final {
             int i = 0;
             auto methodName = primaryMethod.data(gs)->name;
             auto current = primaryMethod;
-            while (current.data(gs)->isOverloaded()) {
+            while (current.data(gs)->flags.isOverloaded) {
                 i++;
                 auto overloadName = gs.lookupNameUnique(core::UniqueNameKind::Overload, methodName, i);
-                auto overload =
-                    primaryMethod.data(gs)->owner.asClassOrModuleRef().data(gs)->findMethod(gs, overloadName);
+                auto overload = primaryMethod.data(gs)->owner.data(gs)->findMethod(gs, overloadName);
                 ENFORCE(overload.exists());
                 if (core::Types::isSubType(gs, intrinsicResultType, overload.data(gs)->resultType)) {
                     return;
@@ -252,17 +251,16 @@ class CallCMethod : public SymbolBasedIntrinsicMethod {
         int i = 0;
         bool acceptsBlock = false;
         auto current = primaryMethod;
-        while (current.data(gs)->isOverloaded()) {
-            const auto &args = current.data(gs)->arguments();
+        while (current.data(gs)->flags.isOverloaded) {
+            const auto &args = current.data(gs)->arguments;
             if (!args.empty() && !args.back().isSyntheticBlockArgument()) {
                 acceptsBlock = true;
                 break;
             }
 
             i++;
             auto overloadName = gs.lookupNameUnique(core::UniqueNameKind::Overload, methodName, i);
-            auto overloadSym =
-                primaryMethod.data(gs)->owner.asClassOrModuleRef().data(gs)->findMember(gs, overloadName);
+            auto overloadSym = primaryMethod.data(gs)->owner.data(gs)->findMember(gs, overloadName);
             ENFORCE(overloadSym.exists());
 
             current = overloadSym.asMethodRef();
@@ -321,7 +319,7 @@ void emitParamInitialization(CompilerState &cs, llvm::IRBuilderBase &builder, co
     InlinedVector<const core::ArgInfo *, 4> keywordArgInfo;
 
     int i = -1;
-    for (auto &argInfo : funcSym.data(cs)->arguments()) {
+    for (auto &argInfo : funcSym.data(cs)->arguments) {
         ++i;
         auto &flags = argInfo.flags;
         if (flags.isBlock) {
@@ -498,7 +496,7 @@ class DefineMethodIntrinsic : public SymbolBasedIntrinsicMethod {
         ENFORCE(funcSym.exists());
 
         // We are going to rely on compiled final methods having their return values checked.
-        const bool needsTypechecking = funcSym.data(cs)->isFinalMethod();
+        const bool needsTypechecking = funcSym.data(cs)->flags.isFinal;
 
         if (methodKind == core::Names::attrReader() && !needsTypechecking) {
             const char *payloadFuncName = isSelf ? "sorbet_defineIvarMethodSingleton" : "sorbet_defineIvarMethod";

diff --git a/core/GlobalState.cc b/core/GlobalState.cc
@@ -978,19 +978,18 @@ TypeArgumentRef GlobalState::enterTypeArgument(Loc loc, MethodRef owner, NameRef
     flags = flags | Symbol::Flags::TYPE_ARGUMENT;
 
     auto ownerScope = owner.dataAllowingNone(*this);
-    histogramInc("symbol_enter_by_name", ownerScope->members().size());
+    histogramInc("symbol_enter_by_name", ownerScope->typeArguments.size());
 
-    auto &store = ownerScope->members()[name];
-    if (store.exists()) {
-        ENFORCE(store.isTypeArgument() && (store.asTypeArgumentRef().data(*this)->flags & flags) == flags,
-                "existing symbol has wrong flags");
-        counterInc("symbols.hit");
-        return store.asTypeArgumentRef();
+    for (auto typeArg : ownerScope->typeArguments) {
+        if (typeArg.dataAllowingNone(*this)->name == name) {
+            ENFORCE((typeArg.dataAllowingNone(*this)->flags & flags) == flags, "existing symbol has wrong flags");
+            counterInc("symbols.hit");
+            return typeArg;
+        }
     }
 
     ENFORCE(!symbolTableFrozen);
     auto result = TypeArgumentRef(*this, typeArguments.size());
-    store = result; // DO NOT MOVE this assignment down. emplace_back on typeArguments invalidates `store`
     typeArguments.emplace_back();
 
     SymbolData data = result.dataAllowingNone(*this);
@@ -1001,7 +1000,7 @@ TypeArgumentRef GlobalState::enterTypeArgument(Loc loc, MethodRef owner, NameRef
     DEBUG_ONLY(categoryCounterInc("symbols", "type_argument"));
     wasModified_ = true;
 
-    owner.dataAllowingNone(*this)->typeArguments().emplace_back(result);
+    owner.dataAllowingNone(*this)->typeArguments.emplace_back(result);
     return result;
 }
 
@@ -1022,9 +1021,8 @@ MethodRef GlobalState::enterMethodSymbol(Loc loc, ClassOrModuleRef owner, NameRe
     store = result; // DO NOT MOVE this assignment down. emplace_back on methods invalidates `store`
     methods.emplace_back();
 
-    SymbolData data = result.dataAllowingNone(*this);
+    MethodData data = result.dataAllowingNone(*this);
     data->name = name;
-    data->flags = Symbol::Flags::METHOD;
     data->owner = owner;
     data->addLoc(*this, loc);
     DEBUG_ONLY(categoryCounterInc("symbols", "method"));
@@ -1038,13 +1036,13 @@ MethodRef GlobalState::enterNewMethodOverload(Loc sigLoc, MethodRef original, co
     NameRef name = num == 0 ? originalName : freshNameUnique(UniqueNameKind::Overload, originalName, num);
     core::Loc loc = num == 0 ? original.data(*this)->loc()
                              : sigLoc; // use original Loc for main overload so that we get right jump-to-def for it.
-    auto owner = original.data(*this)->owner.asClassOrModuleRef();
+    auto owner = original.data(*this)->owner;
     auto res = enterMethodSymbol(loc, owner, name);
     ENFORCE(res != original);
-    if (res.data(*this)->arguments().size() != original.data(*this)->arguments().size()) {
-        ENFORCE(res.data(*this)->arguments().empty());
-        res.data(*this)->arguments().reserve(original.data(*this)->arguments().size());
-        const auto &originalArguments = original.data(*this)->arguments();
+    if (res.data(*this)->arguments.size() != original.data(*this)->arguments.size()) {
+        ENFORCE(res.data(*this)->arguments.empty());
+        res.data(*this)->arguments.reserve(original.data(*this)->arguments.size());
+        const auto &originalArguments = original.data(*this)->arguments;
         int i = -1;
         for (auto &arg : originalArguments) {
             i += 1;
@@ -1131,14 +1129,14 @@ FieldRef GlobalState::enterStaticFieldSymbol(Loc loc, ClassOrModuleRef owner, Na
 ArgInfo &GlobalState::enterMethodArgumentSymbol(Loc loc, MethodRef owner, NameRef name) {
     ENFORCE(owner.exists(), "entering symbol in to non-existing owner");
     ENFORCE(name.exists(), "entering symbol with non-existing name");
-    SymbolData ownerScope = owner.data(*this);
+    MethodData ownerScope = owner.data(*this);
 
-    for (auto &arg : ownerScope->arguments()) {
+    for (auto &arg : ownerScope->arguments) {
         if (arg.name == name) {
             return arg;
         }
     }
-    auto &store = ownerScope->arguments().emplace_back();
+    auto &store = ownerScope->arguments.emplace_back();
 
     ENFORCE(!symbolTableFrozen);
 
@@ -1781,7 +1779,7 @@ unique_ptr<GlobalState> GlobalState::deepCopy(bool keepId) const {
     }
     result->methods.reserve(this->methods.capacity());
     for (auto &sym : this->methods) {
-        result->methods.emplace_back(sym.deepCopy(*result, keepId));
+        result->methods.emplace_back(sym.deepCopy(*result));
     }
     result->fields.reserve(this->fields.capacity());
     for (auto &sym : this->fields) {

diff --git a/core/GlobalState.h b/core/GlobalState.h
@@ -40,6 +40,7 @@ class SerializerImpl;
 class GlobalState final {
     friend NameRef;
     friend Symbol;
+    friend Method;
     friend SymbolRef;
     friend ClassOrModuleRef;
     friend MethodRef;
@@ -298,7 +299,7 @@ class GlobalState final {
     std::vector<UniqueName> uniqueNames;
     UnorderedMap<std::string, FileRef> fileRefByPath;
     std::vector<Symbol> classAndModules;
-    std::vector<Symbol> methods;
+    std::vector<Method> methods;
     std::vector<Symbol> fields;
     std::vector<Symbol> typeMembers;
     std::vector<Symbol> typeArguments;