Replace cjsModuleOffset with segmentID

Summary:
Introduces the concept of a "segment ID" to the runtime and bytecode.

Previously, each BytecodeModule / RuntimeModule had a `cjsModuleOffset` which did double duty as (1) the offset of the first CJS module for registration in the Domain, and (2) a unique ID for each BytecodeModule / RuntimeModule for the purpose of stack traces and source maps.

Here we:

* Remove responsibility (1) because the changes to Domain in D23938175 make it unnecessary.
* Use the input segment ID (from the CommonJS metadata) as the segment ID in the corresponding BytecodeModule / RuntimeModule.
* Rename the resulting concept to `segmentID`.

Reviewed By: avp

Differential Revision: D23940758

fbshipit-source-id: 7f70179a42ca7aceb7c6a5dcfe2ad12141ddfb74

include/hermes/BCGen/HBC/Bytecode.h

@@ -217,10 +217,11 @@ class BytecodeModule {
   /// Object Value Buffer table.
   SerializableBufferTy objValBuffer_;
 
-  /// The ID of the first CJS module in this BytecodeModule.
-  /// This is the first entry in the Domain's CJS module table that will be
-  /// populated by the corresponding RuntimeModule.
-  uint32_t cjsModuleOffset_;
+  /// The segment ID corresponding to this BytecodeModule.
+  /// This uniquely identifies this BytecodeModule within a set of modules
+  /// which were compiled at the same time (and will correspond to a set of
+  /// RuntimeModules in a Domain).
+  uint32_t segmentID_;
 
   /// Table which indicates where to find the different CommonJS modules.
   /// Mapping from {filename ID => function index}.
@@ -260,7 +261,7 @@ class BytecodeModule {
       std::vector<unsigned char> &&arrayBuffer,
       std::vector<unsigned char> &&objKeyBuffer,
       std::vector<unsigned char> &&objValBuffer,
-      uint32_t cjsModuleOffset,
+      uint32_t segmentID,
       std::vector<std::pair<uint32_t, uint32_t>> &&cjsModuleTable,
       std::vector<std::pair<uint32_t, uint32_t>> &&cjsModuleTableStatic,
       BytecodeOptions options)
@@ -274,7 +275,7 @@ class BytecodeModule {
         arrayBuffer_(std::move(arrayBuffer)),
         objKeyBuffer_(std::move(objKeyBuffer)),
         objValBuffer_(std::move(objValBuffer)),
-        cjsModuleOffset_(cjsModuleOffset),
+        segmentID_(segmentID),
         cjsModuleTable_(std::move(cjsModuleTable)),
         cjsModuleTableStatic_(std::move(cjsModuleTableStatic)),
         options_(options) {
@@ -343,8 +344,8 @@ class BytecodeModule {
     return regExpStorage_;
   }
 
-  uint32_t getCJSModuleOffset() const {
-    return cjsModuleOffset_;
+  uint32_t getSegmentID() const {
+    return segmentID_;
   }
 
   llvh::ArrayRef<std::pair<uint32_t, uint32_t>> getCJSModuleTable() const {

include/hermes/BCGen/HBC/BytecodeDataProvider.h

@@ -126,8 +126,8 @@ class BCProviderBase {
   llvh::ArrayRef<RegExpTableEntry> regExpTable_{};
   llvh::ArrayRef<unsigned char> regExpStorage_{};
 
-  /// The ID of the first CJS module in this BytecodeModule.
-  uint32_t cjsModuleOffset_;
+  /// The segment ID corresponding to the bytecode module.
+  uint32_t segmentID_;
 
   /// Table which indicates where to find the different CommonJS modules.
   /// List of unsorted pairs from {filename ID => function index}.
@@ -187,8 +187,8 @@ class BCProviderBase {
   llvh::ArrayRef<unsigned char> getRegExpStorage() const {
     return regExpStorage_;
   }
-  uint32_t getCJSModuleOffset() const {
-    return cjsModuleOffset_;
+  uint32_t getSegmentID() const {
+    return segmentID_;
   }
   llvh::ArrayRef<std::pair<uint32_t, uint32_t>> getCJSModuleTable() const {
     return cjsModuleTable_;

include/hermes/BCGen/HBC/BytecodeFileFormat.h

@@ -30,7 +30,7 @@ const static uint64_t DELTA_MAGIC = ~MAGIC;
 
 // Bytecode version generated by this version of the compiler.
 // Updated: Oct 5, 2020
-const static uint32_t BYTECODE_VERSION = 77;
+const static uint32_t BYTECODE_VERSION = 78;
 
 /// Property cache index which indicates no caching.
 static constexpr uint8_t PROPERTY_CACHING_DISABLED = 0;
@@ -85,7 +85,7 @@ struct BytecodeFileHeader {
   uint32_t arrayBufferSize;
   uint32_t objKeyBufferSize;
   uint32_t objValueBufferSize;
-  uint32_t cjsModuleOffset; // The starting module ID in this segment.
+  uint32_t segmentID; // The ID of this segment.
   uint32_t cjsModuleCount; // Number of modules.
   uint32_t debugInfoOffset;
   BytecodeOptions options;
@@ -111,7 +111,7 @@ struct BytecodeFileHeader {
       uint32_t arrayBufferSize,
       uint32_t objKeyBufferSize,
       uint32_t objValueBufferSize,
-      uint32_t cjsModuleOffset,
+      uint32_t segmentID,
       uint32_t cjsModuleCount,
       uint32_t debugInfoOffset,
       BytecodeOptions options)
@@ -131,7 +131,7 @@ struct BytecodeFileHeader {
         arrayBufferSize(arrayBufferSize),
         objKeyBufferSize(objKeyBufferSize),
         objValueBufferSize(objValueBufferSize),
-        cjsModuleOffset(cjsModuleOffset),
+        segmentID(segmentID),
         cjsModuleCount(cjsModuleCount),
         debugInfoOffset(debugInfoOffset),
         options(options) {

include/hermes/BCGen/HBC/BytecodeGenerator.h

@@ -264,8 +264,8 @@ class BytecodeModuleGenerator {
   /// This allows us to serialize the filenames as part of the debug info.
   UniquingFilenameTable filenameTable_{};
 
-  /// The ID of the first CJS module in this BytecodeModule.
-  uint32_t cjsModuleOffset_{0};
+  /// The ID of this segment.
+  uint32_t segmentID_{0};
 
   /// A record of all the CJS modules registered in this run of generation.
   /// List of pairs: (filename ID, function index).
@@ -352,9 +352,9 @@ class BytecodeModuleGenerator {
   /// \return the index of the string.
   uint32_t addFilename(StringRef str);
 
-  /// Set the CJS module offset for this module.
-  void setCJSModuleOffset(uint32_t offset) {
-    cjsModuleOffset_ = offset;
+  /// Set the segment ID for this module.
+  void setSegmentID(uint32_t id) {
+    segmentID_ = id;
   }
 
   /// Adds a CJS module entry to the table.

include/hermes/BCGen/HBC/DebugInfo.h

@@ -250,7 +250,7 @@ class DebugInfo {
   void populateSourceMap(
       SourceMapGenerator *sourceMap,
       std::vector<uint32_t> &&functionOffsets,
-      uint32_t cjsModuleOffset) const;
+      uint32_t segmentID) const;
 #endif
 };
 

include/hermes/SourceMap/SourceMapGenerator.h

@@ -27,14 +27,14 @@ class SourceMapGenerator {
  public:
   /// Add a line \p line represented as a list of Segments to the 'mappings'
   /// section.
-  /// \param cjsModuleOffset the offset of the module represented by the given
-  /// line, used as the "line" when reporting stack traces from the VM,
-  /// which doesn't have access to the segment IDs.
-  void addMappingsLine(SourceMap::SegmentList line, uint32_t cjsModuleOffset) {
-    if (lines_.size() <= cjsModuleOffset) {
-      lines_.resize(cjsModuleOffset + 1);
+  /// \param segmentID the ID of the segment ( = the BytecodeModule /
+  /// RuntimeModule), used as the "line" when reporting stack traces from the
+  /// VM.
+  void addMappingsLine(SourceMap::SegmentList line, uint32_t segmentID) {
+    if (lines_.size() <= segmentID) {
+      lines_.resize(segmentID + 1);
     }
-    lines_[cjsModuleOffset] = std::move(line);
+    lines_[segmentID] = std::move(line);
   }
 
   /// \return the list of mappings lines.
@@ -66,7 +66,7 @@ class SourceMapGenerator {
   /// SourceMapGenerator::outputAsJSON.
   void addFunctionOffsets(
       std::vector<uint32_t> &&functionOffsets,
-      uint32_t cjsModuleOffset);
+      uint32_t segmentID);
 
   /// Get the source index given the filename.
   uint32_t getSourceIndex(llvh::StringRef filename) const {
@@ -141,7 +141,7 @@ class SourceMapGenerator {
   /// x_facebook_sources field in the JSON source map.
   SourceMap::MetadataList sourcesMetadata_;
 
-  ///  Maps cjsModuleOffset to a vector of function offsets indexed by their
+  ///  Maps segmentID to a vector of function offsets indexed by their
   /// function id.
   llvh::DenseMap<uint32_t, std::vector<uint32_t>> functionOffsets_{};
 };

include/hermes/VM/PredefinedStrings.def

@@ -423,7 +423,7 @@ STR(getFunctionLocation, "getFunctionLocation")
 STR(isNative, "isNative")
 STR(lineNumber, "lineNumber")
 STR(columnNumber, "columnNumber")
-STR(cjsModuleOffset, "cjsModuleOffset")
+STR(segmentID, "segmentID")
 STR(virtualOffset, "virtualOffset")
 STR(fileName, "fileName")
 

lib/BCGen/HBC/Bytecode.cpp

@@ -37,7 +37,7 @@ void BytecodeModule::populateSourceMap(SourceMapGenerator *sourceMap) const {
     offset += func->getHeader().bytecodeSizeInBytes;
   }
   debugInfo_.populateSourceMap(
-      sourceMap, std::move(functionOffsets), cjsModuleOffset_);
+      sourceMap, std::move(functionOffsets), segmentID_);
 }
 
 void BytecodeModule::inlineJumpTables() {

lib/BCGen/HBC/BytecodeDataProvider.cpp

@@ -580,7 +580,7 @@ BCProviderFromBuffer::BCProviderFromBuffer(
   objValueBuffer_ = fields.objValueBuffer;
   regExpTable_ = fields.regExpTable;
   regExpStorage_ = fields.regExpStorage;
-  cjsModuleOffset_ = fileHeader->cjsModuleOffset;
+  segmentID_ = fileHeader->segmentID;
   cjsModuleTable_ = fields.cjsModuleTable;
   cjsModuleTableStatic_ = fields.cjsModuleTableStatic;
 }

lib/BCGen/HBC/BytecodeDisassembler.cpp

@@ -124,8 +124,7 @@ void BytecodeDisassembler::disassembleBytecodeFileHeader(raw_ostream &OS) {
   OS << "  String Kind Entry count: " << bcProvider_->getStringKinds().size()
      << "\n";
   OS << "  RegExp count: " << bcProvider_->getRegExpTable().size() << "\n";
-  OS << "  CommonJS module offset: " << bcProvider_->getCJSModuleOffset()
-     << "\n";
+  OS << "  Segment ID: " << bcProvider_->getSegmentID() << "\n";
   OS << "  CommonJS module count: " << bcProvider_->getCJSModuleTable().size()
      << "\n";
   OS << "  CommonJS module count (static): "

lib/BCGen/HBC/BytecodeGenerator.cpp

@@ -195,10 +195,6 @@ void BytecodeModuleGenerator::addCJSModuleStatic(
     uint32_t moduleID,
     uint32_t functionID) {
   assert(cjsModules_.empty() && "Unresolved modules must be in cjsModules_");
-  assert(
-      moduleID - cjsModuleOffset_ == cjsModulesStatic_.size() &&
-      "Module ID out of order in cjsModulesStatic_");
-  (void)moduleID;
   cjsModulesStatic_.push_back({moduleID, functionID});
 }
 
@@ -230,7 +226,7 @@ std::unique_ptr<BytecodeModule> BytecodeModuleGenerator::generate() {
       std::move(arrayBuffer_),
       std::move(objKeyBuffer_),
       std::move(objValBuffer_),
-      cjsModuleOffset_,
+      segmentID_,
       std::move(cjsModules_),
       std::move(cjsModulesStatic_),
       bytecodeOptions)};

lib/BCGen/HBC/BytecodeProviderFromSrc.cpp

@@ -72,7 +72,7 @@ BCProviderFromSrc::BCProviderFromSrc(
   objKeyBuffer_ = module_->getObjectBuffer().first;
   objValueBuffer_ = module_->getObjectBuffer().second;
 
-  cjsModuleOffset_ = module_->getCJSModuleOffset();
+  segmentID_ = module_->getSegmentID();
   cjsModuleTable_ = module_->getCJSModuleTable();
   cjsModuleTableStatic_ = module_->getCJSModuleTableStatic();
 

lib/BCGen/HBC/BytecodeStream.cpp

@@ -32,7 +32,7 @@ void BytecodeSerializer::serialize(BytecodeModule &BM, const SHA1 &sourceHash) {
                             BM.getArrayBufferSize(),
                             BM.getObjectKeyBufferSize(),
                             BM.getObjectValueBufferSize(),
-                            BM.getCJSModuleOffset(),
+                            BM.getSegmentID(),
                             cjsModuleCount,
                             debugInfoOffset_,
                             BM.getBytecodeOptions()};

lib/BCGen/HBC/DebugInfo.cpp

@@ -322,7 +322,7 @@ void DebugInfo::disassembleLexicalData(llvh::raw_ostream &OS) const {
 void DebugInfo::populateSourceMap(
     SourceMapGenerator *sourceMap,
     std::vector<uint32_t> &&functionOffsets,
-    uint32_t cjsModuleOffset) const {
+    uint32_t segmentID) const {
   // Since our bytecode is not JavaScript, we interpret the source map in a
   // creative way: each bytecode module is represented as a line, and bytecode
   // addresses in the file are represented as column offsets.
@@ -355,8 +355,8 @@ void DebugInfo::populateSourceMap(
       segments.push_back(segmentFor(*loc, offsetInFile, offset));
     offset = fdid.getOffset();
   }
-  sourceMap->addMappingsLine(std::move(segments), cjsModuleOffset);
-  sourceMap->addFunctionOffsets(std::move(functionOffsets), cjsModuleOffset);
+  sourceMap->addMappingsLine(std::move(segments), segmentID);
+  sourceMap->addFunctionOffsets(std::move(functionOffsets), segmentID);
 }
 #endif
 

lib/BCGen/HBC/HBC.cpp

@@ -171,7 +171,7 @@ std::unique_ptr<BytecodeModule> hbc::generateBytecodeModule(
   BytecodeModuleGenerator BMGen(options);
 
   if (range) {
-    BMGen.setCJSModuleOffset(range->first);
+    BMGen.setSegmentID(range->segment);
   }
 
   // Empty if all functions should be generated (i.e. bundle splitting was not

lib/SourceMap/SourceMapGenerator.cpp

@@ -242,9 +242,9 @@ void SourceMapGenerator::outputAsJSONImpl(llvh::raw_ostream &OS) const {
 
 void SourceMapGenerator::addFunctionOffsets(
     std::vector<uint32_t> &&functionOffsets,
-    uint32_t cjsModuleOffset) {
+    uint32_t segmentID) {
   assert(functionOffsets.size() > 0 && "functionOffsets can not be empty");
-  functionOffsets_[cjsModuleOffset] = std::move(functionOffsets);
+  functionOffsets_[segmentID] = std::move(functionOffsets);
 }
 
 } // namespace hermes

lib/VM/JSError.cpp

@@ -607,12 +607,10 @@ ExecutionStatus JSError::constructStackTraceString(
         // Code block was not in the cache, update the cache.
         virtualOffset = sti.codeBlock->getVirtualOffset();
       }
-      // Add 1 to the CJSModuleOffset to account for 1-based indexing of
+      // Add 1 to the SegmentID to account for 1-based indexing of
       // symbolication tools.
-      lineNo = sti.codeBlock->getRuntimeModule()
-                   ->getBytecode()
-                   ->getCJSModuleOffset() +
-          1;
+      lineNo =
+          sti.codeBlock->getRuntimeModule()->getBytecode()->getSegmentID() + 1;
       columnNo = sti.bytecodeOffset + virtualOffset;
       isAddress = true;
     }

lib/VM/JSLib/HermesInternal.cpp

@@ -599,7 +599,7 @@ static CallResult<HermesValue> getCodeBlockFileName(
 /// * fileName (string)
 /// * lineNumber (number) - 1 based
 /// * columnNumber (number) - 1 based
-/// * cjsModuleOffset (number) - 0 based
+/// * segmentID (number) - 0 based
 /// * virtualOffset (number) - 0 based
 /// * isNative (boolean)
 /// TypeError if func is not a function.
@@ -651,15 +651,14 @@ hermesInternalGetFunctionLocation(void *, Runtime *runtime, NativeArgs args) {
       (void)res;
     } else {
       tmpHandle = HermesValue::encodeNumberValue(
-          codeBlock->getRuntimeModule()->getBytecode()->getCJSModuleOffset());
+          codeBlock->getRuntimeModule()->getBytecode()->getSegmentID());
       res = JSObject::defineOwnProperty(
           resultHandle,
           runtime,
-          Predefined::getSymbolID(Predefined::cjsModuleOffset),
+          Predefined::getSymbolID(Predefined::segmentID),
           DefinePropertyFlags::getDefaultNewPropertyFlags(),
           tmpHandle);
-      assert(
-          res != ExecutionStatus::EXCEPTION && "Failed to set cjsModuleOffset");
+      assert(res != ExecutionStatus::EXCEPTION && "Failed to set segmentID");
       (void)res;
 
       tmpHandle = HermesValue::encodeNumberValue(codeBlock->getVirtualOffset());

lib/VM/Profiler/CodeCoverageProfiler.cpp

@@ -58,13 +58,13 @@ CodeCoverageProfiler::getExecutedFunctions() {
   }
   for (auto &entry : coverageInfoIter->second.executedFuncBitsArrayMap) {
     auto *bcProvider = entry.first->getBytecode();
-    const uint32_t moduleId = bcProvider->getCJSModuleOffset();
+    const uint32_t segmentID = bcProvider->getSegmentID();
     const std::vector<bool> &moduleFuncBitsArray = entry.second;
     for (uint32_t i = 0; i < moduleFuncBitsArray.size(); ++i) {
       if (moduleFuncBitsArray[i]) {
         const uint32_t funcVirtualOffset =
             bcProvider->getVirtualOffsetForFunction(i);
-        funcInfos.emplace_back(moduleId, funcVirtualOffset);
+        funcInfos.emplace_back(segmentID, funcVirtualOffset);
       }
     }
   }

lib/VM/Profiler/SamplingProfilerPosix.cpp

@@ -363,8 +363,8 @@ uint32_t SamplingProfiler::walkRuntimeStack(
                                      stackFrame.jsFrame.functionId) +
             stackFrame.jsFrame.offset;
 
-        uint32_t moduleId = bcProvider->getCJSModuleOffset();
-        uint64_t frameAddress = ((uint64_t)moduleId << 32) + virtualOffset;
+        uint32_t segmentID = bcProvider->getSegmentID();
+        uint64_t frameAddress = ((uint64_t)segmentID << 32) + virtualOffset;
         assert(
             (frameAddress & kNativeFrameMask) == 0 &&
             "Module id should take less than 32 bits");

lib/VM/Runtime.cpp

@@ -1762,10 +1762,9 @@ void Runtime::crashWriteCallStack(JSONEmitter &json) {
                   .toStringRef(srcLocStorage));
         }
       }
-      uint32_t cjsModuleOffset =
-          runtimeModule->getBytecode()->getCJSModuleOffset();
+      uint32_t segmentID = runtimeModule->getBytecode()->getSegmentID();
       llvh::StringRef sourceURL = runtimeModule->getSourceURL();
-      json.emitKeyValue("CJSModuleOffset", cjsModuleOffset);
+      json.emitKeyValue("SegmentID", segmentID);
       json.emitKeyValue("SourceURL", sourceURL);
     } else {
       json.emitKeyValue("NativeCode", true);