Directly store SymbolID in JSSymbol

Summary:
`SymbolID` has more efficient write barriers and is cheaper to mark
than `GCSmallHermesValue`

Reviewed By: kodafb

Differential Revision: D28889939

fbshipit-source-id: 11d4a7bc2b9d1790cf02655b220b134a3f39938a

include/hermes/VM/PrimitiveBox.h

@@ -300,7 +300,9 @@ class JSBoolean final : public JSObject {
 };
 
 /// Symbol object.
-class JSSymbol final : public PrimitiveBox {
+class JSSymbol final : public JSObject {
+  friend void SymbolObjectBuildMeta(const GCCell *cell, Metadata::Builder &mb);
+
  public:
   static const ObjectVTable vt;
 
@@ -317,19 +319,24 @@ class JSSymbol final : public PrimitiveBox {
     return create(runtime, SymbolID{}, prototype);
   }
 
-  /// Return the [[PrimitiveValue]] internal property as a string.
-  static const PseudoHandle<SymbolID> getPrimitiveSymbol(JSObject *self) {
-    return PseudoHandle<SymbolID>::create(getPrimitiveValue(self).getSymbol());
+  /// Return the [[PrimitiveValue]] internal property as a SymbolID.
+  PseudoHandle<SymbolID> getPrimitiveSymbol() const {
+    return PseudoHandle<SymbolID>::create(primitiveValue_);
   }
 
 #ifdef HERMESVM_SERIALIZE
   explicit JSSymbol(Deserializer &d);
-
-  friend void SymbolObjectDeserialize(Deserializer &d, CellKind kind);
 #endif
 
-  JSSymbol(Runtime *runtime, Handle<JSObject> parent, Handle<HiddenClass> clazz)
-      : PrimitiveBox(runtime, &vt.base, *parent, *clazz) {}
+  JSSymbol(
+      Runtime *runtime,
+      SymbolID value,
+      Handle<JSObject> parent,
+      Handle<HiddenClass> clazz)
+      : JSObject(runtime, &vt.base, *parent, *clazz), primitiveValue_(value) {}
+
+ private:
+  const GCSymbolID primitiveValue_;
 };
 
 } // namespace vm

lib/VM/JSLib/Symbol.cpp

@@ -242,7 +242,7 @@ symbolPrototypeDescriptionGetter(void *, Runtime *runtime, NativeArgs args) {
   if (args.getThisArg().isSymbol()) {
     sym = args.vmcastThis<SymbolID>().get();
   } else if (auto symHandle = args.dyncastThis<JSSymbol>()) {
-    sym = JSSymbol::getPrimitiveSymbol(*symHandle).get();
+    sym = symHandle->getPrimitiveSymbol();
   } else {
     return runtime->raiseTypeError(
         "Symbol.prototype.description can only be called on Symbol");
@@ -261,7 +261,7 @@ symbolPrototypeToString(void *, Runtime *runtime, NativeArgs args) {
   if (args.getThisArg().isSymbol()) {
     sym = args.vmcastThis<SymbolID>().get();
   } else if (auto symHandle = args.dyncastThis<JSSymbol>()) {
-    sym = JSSymbol::getPrimitiveSymbol(*symHandle).get();
+    sym = symHandle->getPrimitiveSymbol();
   } else {
     return runtime->raiseTypeError(
         "Symbol.prototype.toString can only be called on Symbol");
@@ -283,7 +283,7 @@ symbolPrototypeValueOf(void *, Runtime *runtime, NativeArgs args) {
     return args.getThisArg();
   }
   if (auto jsSymbol = args.dyncastThis<JSSymbol>()) {
-    return JSSymbol::getPrimitiveSymbol(*jsSymbol).getHermesValue();
+    return jsSymbol->getPrimitiveSymbol().getHermesValue();
   }
   return runtime->raiseTypeError(
       "Symbol.prototype.valueOf can only be called on Symbol");

lib/VM/PrimitiveBox.cpp

@@ -425,14 +425,20 @@ const ObjectVTable JSSymbol::vt{
 void SymbolObjectBuildMeta(const GCCell *cell, Metadata::Builder &mb) {
   mb.addJSObjectOverlapSlots(JSObject::numOverlapSlots<JSSymbol>());
   ObjectBuildMeta(cell, mb);
+  const auto *self = static_cast<const JSSymbol *>(cell);
   mb.setVTable(&JSSymbol::vt.base);
+  mb.addField(&self->primitiveValue_);
 }
 
 #ifdef HERMESVM_SERIALIZE
-JSSymbol::JSSymbol(Deserializer &d) : PrimitiveBox(d, &vt.base) {}
+JSSymbol::JSSymbol(Deserializer &d)
+    : JSObject(d, &vt.base),
+      primitiveValue_(SymbolID::unsafeCreate(d.readInt<SymbolID::RawType>())) {}
 
 void SymbolObjectSerialize(Serializer &s, const GCCell *cell) {
   JSObject::serializeObjectImpl(s, cell, JSObject::numOverlapSlots<JSSymbol>());
+  const auto *self = static_cast<const JSSymbol *>(cell);
+  s.writeInt<uint32_t>(self->getPrimitiveSymbol().get().unsafeGetRaw());
   s.endObject(cell);
 }
 
@@ -449,15 +455,9 @@ PseudoHandle<JSSymbol> JSSymbol::create(
     Handle<JSObject> parentHandle) {
   auto clazzHandle = runtime->getHiddenClassForPrototype(
       *parentHandle, numOverlapSlots<JSSymbol>() + ANONYMOUS_PROPERTY_SLOTS);
-  auto *obj = runtime->makeAFixed<JSSymbol>(runtime, parentHandle, clazzHandle);
-  auto self = JSObjectInit::initToPseudoHandle(runtime, obj);
-
-  JSObject::setDirectSlotValue<PrimitiveBox::primitiveValuePropIndex()>(
-      self.get(),
-      SmallHermesValue::encodeSymbolValue(value),
-      &runtime->getHeap());
-
-  return self;
+  auto *obj =
+      runtime->makeAFixed<JSSymbol>(runtime, value, parentHandle, clazzHandle);
+  return JSObjectInit::initToPseudoHandle(runtime, obj);
 }
 
 } // namespace vm