VM startup performance fixes, -O3 Android release, and two upstream backports

CMakeLists.txt

@@ -364,6 +364,17 @@ if(HAVE_ICF)
   list(APPEND HERMES_EXTRA_LINKER_FLAGS "LINKER:--icf=safe")
 endif()
 
+set(HERMES_EXTRA_COMPILE_FLAGS "" CACHE STRING "Extra compile flags (space separated)")
+if(HERMES_EXTRA_COMPILE_FLAGS)
+  separate_arguments(_hermes_extra_compile_flags UNIX_COMMAND "${HERMES_EXTRA_COMPILE_FLAGS}")
+  add_compile_options(${_hermes_extra_compile_flags})
+endif()
+set(HERMES_EXTRA_LINK_FLAGS "" CACHE STRING "Extra linker flags for Hermes binaries/libraries (space separated)")
+if(HERMES_EXTRA_LINK_FLAGS)
+  separate_arguments(_hermes_extra_link_flags UNIX_COMMAND "${HERMES_EXTRA_LINK_FLAGS}")
+  list(APPEND HERMES_EXTRA_LINKER_FLAGS ${_hermes_extra_link_flags})
+endif()
+
 # Make the HERMES_RELEASE_VERSION accessible for version printing in C++.
 if(HERMES_RELEASE_VERSION)
     add_definitions(-DHERMES_RELEASE_VERSION="${HERMES_RELEASE_VERSION}")

android/build.gradle.kts

@@ -264,7 +264,16 @@ android {
         }
       }
     }
-    release { externalNativeBuild { cmake { arguments("-DCMAKE_BUILD_TYPE=MinSizeRel") } } }
+    release {
+      externalNativeBuild {
+        cmake {
+          val releaseBuildType = project.findProperty("hermesReleaseCmakeBuildType")?.toString() ?: "Release"
+          arguments("-DCMAKE_BUILD_TYPE=$releaseBuildType")
+          project.findProperty("hermesHeapHvMode")?.toString()?.let { arguments("-DHERMESVM_HEAP_HV_MODE=$it") }
+          project.findProperty("hermesExtraCmakeArgs")?.toString()?.split("|")?.filter { it.isNotBlank() }?.forEach { arguments(it) }
+        }
+      }
+    }
 
     buildTypes {
       create("debugOptimized") {

include/hermes/VM/HiddenClass.h

@@ -55,6 +55,11 @@ struct ClassFlags {
   /// property is set and then deleted, this will still be set to true.
   uint8_t mayHaveAccessor : 1;
 
+  /// Number of times this dictionary has been re-classed into a new
+  /// *cacheable* dictionary on property delete/update (see
+  /// HiddenClass::kDictReclassLimit).
+  uint8_t dictReclassCount : 4;
+
   ClassFlags() {
     ::memset(this, 0, sizeof(*this));
   }
@@ -399,6 +404,19 @@ class HiddenClass final : public GCCell {
   /// false. If there was no allocated property map, this returns llvh::None. If
   /// this fails by returning None, the "slow path", \c findProperty() itself,
   /// must be used.
+  /// \return the existing child of \p self for adding property \p name with
+  /// \p flags, or nullptr. A non-null result implies \p self does not have
+  /// \p name. Does not allocate.
+  static HiddenClass *findExistingAddTransition(
+      HiddenClass *self,
+      Runtime &runtime,
+      SymbolID name,
+      PropertyFlags flags) {
+    if (self->isDictionary())
+      return nullptr;
+    return self->transitionMap_.lookup(runtime, {name, flags});
+  }
+
   static OptValue<bool> tryFindPropertyFast(
       const HiddenClass *self,
       PointerBase &base,
@@ -518,6 +536,23 @@ class HiddenClass final : public GCCell {
   /// will have one.  The \p noCache argument indicates whether the
   /// new dictionary \c HiddenClass's dictionaryNoCacheMode flag will
   /// be set.  \return the new class.
+  /// Deleting or updating a property of a cacheable dictionary must give the
+  /// object a new HiddenClass identity so property caches keyed by the old one
+  /// can't hit. Upstream allowed this once and then switched the dictionary to
+  /// no-cache mode forever, which makes every property-cache lookup on it miss
+  /// (notably the global object, whose lazy globals are redefined at startup).
+  /// Allow up to this many cacheable re-classings first.
+  static constexpr unsigned kDictReclassLimit = 15;
+  static_assert(
+      kDictReclassLimit < (1u << 4),
+      "kDictReclassLimit must fit in ClassFlags::dictReclassCount");
+
+  /// If \p self is a cacheable dictionary within kDictReclassLimit, return a
+  /// new cacheable dictionary class; otherwise a null handle.
+  static Handle<HiddenClass> tryReclassCacheableDictionary(
+      Handle<HiddenClass> self,
+      Runtime &runtime);
+
   static Handle<HiddenClass> copyToNewDictionary(
       Handle<HiddenClass> selfHandle,
       Runtime &runtime,

include/hermes/VM/IdentifierTable.h

@@ -8,6 +8,7 @@
 #ifndef HERMES_VM_IDENTIFIERTABLE_H
 #define HERMES_VM_IDENTIFIERTABLE_H
 
+#include <cstring>
 #include "hermes/ADT/PtrOrInt.h"
 #include "hermes/ADT/TransparentConservativeVector.h"
 #include "hermes/Support/HashString.h"
@@ -180,6 +181,12 @@ class IdentifierTable {
     return lookupVector_.size();
   }
 
+  /// Forget all cached (non-uniqued StringPrimitive* -> SymbolID) mappings.
+  /// Must be called whenever objects may move or symbols may be freed.
+  void clearStrSymCache() {
+    memset(strSymCache_, 0, sizeof(strSymCache_));
+  }
+
   /// Invoked at the end of a GC to free all unmarked symbols. The function may
   /// set additional bits in \p markedSymbols to reflect the fact that some
   /// symbols were not freed.
@@ -212,6 +219,23 @@ class IdentifierTable {
       Handle<StringPrimitive> desc);
 
  private:
+  /// Small direct-mapped cache from non-uniqued string primitives (by address)
+  /// to their SymbolID, so repeated computed-property accesses with the same
+  /// key string (e.g. obj[id] with an id taken from JSON) skip hashing and the
+  /// identifier-table probe. Keyed by address, so it is only valid between
+  /// GCs: cleared by clearStrSymCache() at the start of every GC cycle and
+  /// when symbols are freed.
+  struct StrSymCacheEntry {
+    const StringPrimitive *str;
+    SymbolID sym;
+  };
+  static constexpr unsigned kStrSymCacheSize = 256;
+  StrSymCacheEntry strSymCache_[kStrSymCacheSize]{};
+  static unsigned strSymCacheIndex(const StringPrimitive *str) {
+    auto p = reinterpret_cast<uintptr_t>(str);
+    return ((p >> 3) ^ (p >> 11)) & (kStrSymCacheSize - 1);
+  }
+
   /// Entry in the lookup vector. An entry can represent 5 different types:
   /// 1. A lazy ASCII identifier: asciiPtr_ will be set to a pointer to
   ///   some static char * memory, num_ will be the string length,

include/hermes/VM/JSObject.h

@@ -848,6 +848,18 @@ class JSObject : public GCCell {
   /// proxy is undefined, then false will be returned, otherwise, desc will be
   /// filled in with the result, and true will be returned.  If selfHandle is
   /// not a proxy, then the flag is irrelevant.
+  /// Fast path for object spread (`{...source}` into a fresh `{}`): if
+  /// \p target is an empty, extensible plain object and \p source is a plain
+  /// object whose own properties are all enumerable, writable, configurable,
+  /// string-keyed data properties (non-dictionary class, no accessors, no
+  /// index-like names) with the same prototype, make \p target share
+  /// \p source's hidden class and copy the slot values in bulk. Observably
+  /// identical to CopyDataProperties. \return true if done.
+  static CallResult<bool> tryCloneDataPropertiesForSpread(
+      Handle<JSObject> target,
+      Handle<JSObject> source,
+      Runtime &runtime);
+
   static CallResult<bool> getOwnComputedPrimitiveDescriptor(
       Handle<JSObject> selfHandle,
       Runtime &runtime,

include/hermes/VM/detail/IdentifierHashTable.h

@@ -56,9 +56,19 @@ class IdentifierHashTable {
     return cap - (cap >> 2) < nonEmptyEntryCount_;
   }
 
-  /// Grow the hash table and rehash with \p newCapacity.
+  /// Grow the hash table to \p newCapacity and rehash into it. Aborts unless
+  /// \p newCapacity is strictly greater than the current capacity; this also
+  /// guards against a wrapped-around computation in the caller (e.g. an
+  /// overflowing capacity() * 2 or NextPowerOf2()).
   void growAndRehash(uint32_t newCapacity);
 
+  /// Rehash all valid entries into a freshly allocated table of \p newCapacity,
+  /// dropping deleted entries. \p newCapacity must be a power of two that is
+  /// greater than or equal to the current capacity. Unlike growAndRehash() an
+  /// equal capacity is allowed (in-place compaction), so this must not be used
+  /// where an overflow check is required.
+  void rehash(uint32_t newCapacity);
+
   /// HashIteratorWrapper is a thin wrapper around char* and char16_t*
   /// to make sure that when iterating on it, *itr always return a
   /// char16_t type element. This ensures consistency in hash function.

lib/BCGen/HBC/ISel.cpp

@@ -1151,9 +1151,10 @@ void HBCISel::generateDefineOwnInDenseArrayInst(
   uint32_t index = arrayIndex->asUInt32();
   if (index <= UINT8_MAX) {
     BCFGen_->emitDefineOwnInDenseArray(objReg, valueReg, index);
-  } else if (index <= UINT16_MAX) {
-    // Dense arrays can only be UINT16_MAX because NewArray doesn't accept
-    // lengths over UINT16_MAX.
+  } else if (index < UINT16_MAX) {
+    // NewArray caps the storage at UINT16_MAX elements (indices
+    // 0..UINT16_MAX-1), so index UINT16_MAX must take the growing
+    // DefineOwnByIndex path.
     BCFGen_->emitDefineOwnInDenseArrayL(objReg, valueReg, index);
   } else {
     // Fallback: use DefineOwnByIndex.

lib/VM/HiddenClass.cpp

@@ -193,6 +193,18 @@ CallResult<HermesValue> HiddenClass::create(
   return HermesValue::encodeObjectValue(obj);
 }
 
+Handle<HiddenClass> HiddenClass::tryReclassCacheableDictionary(
+    Handle<HiddenClass> self,
+    Runtime &runtime) {
+  if (!self->isDictionary() || self->isDictionaryNoCache() ||
+      self->flags_.dictReclassCount >= kDictReclassLimit)
+    return Runtime::makeNullHandle<HiddenClass>();
+  unsigned count = self->flags_.dictReclassCount;
+  auto newHandle = copyToNewDictionary(self, runtime, /*noCache*/ false);
+  newHandle->flags_.dictReclassCount = count + 1;
+  return newHandle;
+}
+
 Handle<HiddenClass> HiddenClass::copyToNewDictionary(
     Handle<HiddenClass> selfHandle,
     Runtime &runtime,
@@ -344,10 +356,15 @@ Handle<HiddenClass> HiddenClass::deleteProperty(
     PropertyPos pos) {
   // We convert to dictionary if we're not yet a dictionary
   // (transition to a cacheable dictionary), or if we are, but not yet
-  // in no-cache mode (transition to no-cache mode).
-  auto newHandle = LLVM_UNLIKELY(!selfHandle->isDictionaryNoCache())
-      ? copyToNewDictionary(selfHandle, runtime, selfHandle->isDictionary())
-      : selfHandle;
+  // in no-cache mode (a new cacheable dictionary while within the re-class
+  // budget, otherwise transition to no-cache mode).
+  Handle<HiddenClass> newHandle = selfHandle;
+  if (LLVM_UNLIKELY(!selfHandle->isDictionaryNoCache())) {
+    newHandle = tryReclassCacheableDictionary(selfHandle, runtime);
+    if (!newHandle.get())
+      newHandle = copyToNewDictionary(
+          selfHandle, runtime, selfHandle->isDictionary());
+  }
 
   --newHandle->numProperties_;
 
@@ -514,9 +531,13 @@ Handle<HiddenClass> HiddenClass::updateProperty(
     DictPropertyMap::getDescriptorPair(
         selfHandle->propertyMap_.getNonNull(runtime), pos)
         ->second.flags = newFlags;
-    // If it's still cacheable, make it non-cacheable.
+    // If it's still cacheable, give it a new cacheable class while within the
+    // re-class budget, otherwise make it non-cacheable.
     if (!selfHandle->isDictionaryNoCache()) {
-      selfHandle = copyToNewDictionary(selfHandle, runtime, /*noCache*/ true);
+      auto reclassed = tryReclassCacheableDictionary(selfHandle, runtime);
+      selfHandle = reclassed.get()
+          ? reclassed
+          : copyToNewDictionary(selfHandle, runtime, /*noCache*/ true);
     }
     return selfHandle;
   }

lib/VM/IdentifierTable.cpp

@@ -95,15 +95,27 @@ CallResult<Handle<SymbolID>> IdentifierTable::getSymbolHandleFromPrimitive(
     SymbolID id = str->getUniqueID(runtime);
     return runtime.makeHandle(id);
   }
-  auto handle = runtime.makeHandle(std::move(str));
+  StrSymCacheEntry *cacheEntry = &strSymCache_[strSymCacheIndex(str.get())];
+  if (cacheEntry->str == str.get()) {
+    runtime.getHeap().weakRefReadBarrier(cacheEntry->sym);
+    return runtime.makeHandle(cacheEntry->sym);
+  }
+  auto origHandle = runtime.makeHandle(std::move(str));
   // Force the string primitive to flatten if it's a rope.
-  handle = StringPrimitive::ensureFlat(runtime, handle);
+  auto handle = StringPrimitive::ensureFlat(runtime, origHandle);
   uint32_t hash = handle->getOrComputeHash();
   auto cr = handle->isASCII()
       ? getOrCreateIdentifier(runtime, handle->castToASCIIRef(), handle, hash)
       : getOrCreateIdentifier(runtime, handle->castToUTF16Ref(), handle, hash);
   if (LLVM_UNLIKELY(cr == ExecutionStatus::EXCEPTION))
     return ExecutionStatus::EXCEPTION;
+  if (!origHandle->isUniqued()) {
+    // Re-read the address through the handle: the calls above may have
+    // allocated and run a GC (which clears the cache and may move strings).
+    cacheEntry = &strSymCache_[strSymCacheIndex(origHandle.get())];
+    cacheEntry->str = origHandle.get();
+    cacheEntry->sym = *cr;
+  }
   return runtime.makeHandle(*cr);
 }
 
@@ -490,6 +502,7 @@ void IdentifierTable::freeID(uint32_t id) {
 void IdentifierTable::freeUnmarkedSymbols(
     llvh::BitVector &markedSymbols,
     GC::IDTracker &tracker) {
+  clearStrSymCache();
   assert(
       markedSymbols.size() <= lookupVector_.size() &&
       "Size of markedSymbols must be less than the current lookupVector");

lib/VM/JSLib/HermesBuiltin.cpp

@@ -354,6 +354,16 @@ CallResult<HermesValue> hermesBuiltinCopyDataProperties(
       (!excludedItems || !excludedItems->isProxyObject()) &&
       "excludedItems internal List is a Proxy");
 
+  // Spreading into a fresh {}: share the source's hidden class.
+  if (!excludedItems) {
+    auto cloned =
+        JSObject::tryCloneDataPropertiesForSpread(target, source, runtime);
+    if (LLVM_UNLIKELY(cloned == ExecutionStatus::EXCEPTION))
+      return ExecutionStatus::EXCEPTION;
+    if (*cloned)
+      return target.getHermesValue();
+  }
+
   // We cannot use the fast path if the object is a proxy, host object, or when
   // there could potentially be an accessor defined on the object. This is
   // because in order to use JSObject::forEachOwnPropertyWhile, we must not