From 56c5d4e8105e7476258cf5c93408e4b596043e78 Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Sun, 26 May 2024 07:22:22 -0500 Subject: [PATCH] Cache the class path Symbol on the class Additional improvements here: * rubyName no longer reverses the parents list before iterating. * Factored out common path string-building logic. * Smaller initial parents list (more than 5 parents is very rare). --- core/src/main/java/org/jruby/RubyModule.java | 87 ++++++++++++++++--- .../org/jruby/runtime/marshal/NewMarshal.java | 12 +-- 2 files changed, 80 insertions(+), 19 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java index 83ac483c068..2206e63efae 100644 --- a/core/src/main/java/org/jruby/RubyModule.java +++ b/core/src/main/java/org/jruby/RubyModule.java @@ -654,8 +654,7 @@ public void setBaseName(String name) { * @return The generated class name */ public String getName() { - if (cachedName != null) return cachedName; - return calculateName(); + return cachedName != null ? cachedName : calculateName(); } /** @@ -670,6 +669,24 @@ public RubyString rubyName() { return cachedRubyName != null ? cachedRubyName : calculateRubyName(); } + /** + * Generate a fully-qualified class name or a #-style name as a Symbol. If any element of the path is anonymous, + * returns null. + * + * @return a properly encoded non-anonymous class path as a Symbol, or null. + */ + public RubySymbol symbolName() { + IRubyObject symbolName = cachedSymbolName; + + if (symbolName == null) { + cachedSymbolName = symbolName = calculateSymbolName(); + } + + if (symbolName == UNDEF) return null; + + return (RubySymbol) symbolName; + } + public RubyString rubyBaseName() { String baseName = getBaseName(); @@ -691,16 +708,18 @@ private RubyString calculateRubyName() { if (getBaseName() == null) return calculateAnonymousRubyName(); // no name...anonymous! + Ruby runtime = getRuntime(); + if (usingTemporaryName()) { // temporary name - cachedRubyName = getRuntime().newSymbol(baseName).toRubyString(getRuntime().getCurrentContext()); + cachedRubyName = runtime.newSymbol(baseName).toRubyString(getRuntime().getCurrentContext()); return cachedRubyName; } - - Ruby runtime = getRuntime(); + RubyClass objectClass = runtime.getObject(); - List parents = new ArrayList<>(); - for (RubyModule p = getParent(); p != null && p != objectClass; p = p.getParent()) { - if (p == this) break; // Break out of cyclic namespaces like C::A = C2; C2::A = C (jruby/jruby#2314) + List parents = new ArrayList<>(5); + for (RubyModule p = getParent(); + p != null && p != objectClass && p != this; // Break out of cyclic namespaces like C::A = C2; C2::A = C (jruby/jruby#2314) + p = p.getParent()) { RubyString name = p.rubyBaseName(); @@ -717,21 +736,60 @@ private RubyString calculateRubyName() { parents.add(name); } - Collections.reverse(parents); + RubyString fullName = buildPathString(runtime, parents); + + if (cache) cachedRubyName = fullName; + return fullName; + } + + /** + * Calculate the module's name path as a Symbol. If any element in the path is anonymous, this will return null. + * + * Used primarily by Marshal.dump for class names. + * + * @return a non-anonymous class path as a Symbol, or null + */ + private IRubyObject calculateSymbolName() { + if (getBaseName() == null) return UNDEF; + + Ruby runtime = getRuntime(); + + if (usingTemporaryName()) { // temporary name + return runtime.newSymbol(baseName); + } + + RubyClass objectClass = runtime.getObject(); + List parents = new ArrayList<>(5); + for (RubyModule p = getParent(); + p != null && p != objectClass && p != this; // Break out of cyclic namespaces like C::A = C2; C2::A = C (jruby/jruby#2314) + p = p.getParent()) { + + RubyString name = p.rubyBaseName(); + + if (name == null) { + return UNDEF; + } + + parents.add(name); + } + + RubyString fullName = buildPathString(runtime, parents); + + return fullName.intern(); + } + + private RubyString buildPathString(Ruby runtime, List parents) { RubyString colons = runtime.newString("::"); RubyString fullName = runtime.newString(); // newString creates empty ByteList which ends up as fullName.setEncoding(USASCIIEncoding.INSTANCE); // ASCII-8BIT. 8BIT is unfriendly to string concats. - for (RubyString parent: parents) { - RubyString rubyString = fullName.catWithCodeRange(parent); + for (int i = parents.size() - 1; i >= 0; i--) { + RubyString rubyString = fullName.catWithCodeRange(parents.get(i)); rubyString.catWithCodeRange(colons); } fullName.catWithCodeRange(rubyBaseName()); fullName.setFrozen(true); - - if (cache) cachedRubyName = fullName; - return fullName; } @@ -5779,6 +5837,7 @@ public IRubyObject initialize(Block block) { */ private transient String cachedName; private transient RubyString cachedRubyName; + private transient IRubyObject cachedSymbolName; @SuppressWarnings("unchecked") private volatile Map constants = Collections.EMPTY_MAP; diff --git a/core/src/main/java/org/jruby/runtime/marshal/NewMarshal.java b/core/src/main/java/org/jruby/runtime/marshal/NewMarshal.java index d1c31db71a4..0657e4af008 100644 --- a/core/src/main/java/org/jruby/runtime/marshal/NewMarshal.java +++ b/core/src/main/java/org/jruby/runtime/marshal/NewMarshal.java @@ -278,14 +278,16 @@ private void dumpType(ThreadContext context, RubyOutputStream out, IRubyObject v } } - public static String getPathFromClass(ThreadContext context, RubyModule clazz) { + public static RubySymbol getPathFromClass(ThreadContext context, RubyModule clazz) { Ruby runtime = context.runtime; - RubyString path = clazz.rubyName(); + RubySymbol pathSym = clazz.symbolName(); - if (path.charAt(0) == '#') { + if (pathSym == null) { String type = clazz.isClass() ? "class" : "module"; throw typeError(context, str(runtime, "can't dump anonymous " + type + " ", types(runtime, clazz))); } + + String path = pathSym.idString(); RubyModule real = clazz.isModule() ? clazz : ((RubyClass)clazz).getRealClass(); @@ -293,10 +295,10 @@ public static String getPathFromClass(ThreadContext context, RubyModule clazz) { // will this fail on? If there is a failing case then passing asJavaString may be broken since it will not be // a properly encoded string. If this is an issue we should make a clazz.IdPath where all segments are returned // by their id names. - if (runtime.getClassFromPath(path.asJavaString()) != real) { + if (runtime.getClassFromPath(path) != real) { throw typeError(context, str(runtime, types(runtime, clazz), " can't be referred")); } - return path.asJavaString(); + return pathSym; } private void writeObjectData(ThreadContext context, RubyOutputStream out, IRubyObject value) {