Bug 1622316 - Add buffering to the PlatformWriter to reduce the number of syscalls required to write the .extra file r=KrisWright
authorGabriele Svelto <gsvelto@mozilla.com>
Tue, 24 Mar 2020 11:17:23 +0000
changeset 520200 88126ed3e7603cf446b4af93c0fa1743af4cb75a
parent 520199 b9862f5a69d44db8f06c2aae3f351227320d2840
child 520201 c1c84967fb5de818f98b4e6707d919ae18a9113a
push id37245
push useropoprus@mozilla.com
push dateTue, 24 Mar 2020 21:46:41 +0000
treeherdermozilla-central@dbabf2e388fa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersKrisWright
bugs1622316
milestone76.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1622316 - Add buffering to the PlatformWriter to reduce the number of syscalls required to write the .extra file r=KrisWright Differential Revision: https://phabricator.services.mozilla.com/D67588
toolkit/crashreporter/nsExceptionHandler.cpp
--- a/toolkit/crashreporter/nsExceptionHandler.cpp
+++ b/toolkit/crashreporter/nsExceptionHandler.cpp
@@ -546,23 +546,24 @@ class PlatformWriter {
 
   const NativeFileDesc kInvalidFileDesc =
 #ifdef XP_WIN
       INVALID_HANDLE_VALUE;
 #elif defined(XP_UNIX)
       -1;
 #endif
 
-  PlatformWriter() : mFD(kInvalidFileDesc) {}
+  PlatformWriter() : mBuffer{}, mPos(0), mFD(kInvalidFileDesc) {}
   explicit PlatformWriter(const NativeChar* aPath) : PlatformWriter() {
     Open(aPath);
   }
 
   ~PlatformWriter() {
     if (Valid()) {
+      Flush();
 #ifdef XP_WIN
       CloseHandle(mFD);
 #elif defined(XP_UNIX)
       sys_close(mFD);
 #endif
     }
   }
 
@@ -577,34 +578,60 @@ class PlatformWriter {
 
   void OpenHandle(NativeFileDesc aFD) { mFD = aFD; }
   bool Valid() { return mFD != kInvalidFileDesc; }
 
   void WriteBuffer(const char* aBuffer, size_t aLen) {
     if (!Valid()) {
       return;
     }
-#ifdef XP_WIN
-    DWORD nBytes;
-    WriteFile(mFD, aBuffer, aLen, &nBytes, nullptr);
-#elif defined(XP_UNIX)
-    mozilla::Unused << sys_write(mFD, aBuffer, aLen);
-#endif
+
+    while (aLen-- > 0) {
+      WriteChar(*aBuffer++);
+    }
   }
 
   void WriteString(const char* aStr) { WriteBuffer(aStr, my_strlen(aStr)); }
 
   template <int N>
   void WriteLiteral(const char (&aStr)[N]) {
     WriteBuffer(aStr, N - 1);
   }
 
   NativeFileDesc FileDesc() { return mFD; }
 
  private:
+  DISALLOW_COPY_AND_ASSIGN(PlatformWriter);
+
+  void WriteChar(char aChar) {
+    if (mPos == kBufferSize) {
+      Flush();
+    }
+
+    mBuffer[mPos++] = aChar;
+  }
+
+  void Flush() {
+    if (mPos > 0) {
+#ifdef XP_WIN
+      DWORD nBytes = 0;
+      while (nBytes < mPos) {
+        WriteFile(mFD, mBuffer, mPos, &nBytes, nullptr);
+      }
+#elif defined(XP_UNIX)
+      mozilla::Unused << sys_write(mFD, mBuffer, mPos);
+#endif
+      mPos = 0;
+    }
+  }
+
+  static const size_t kBufferSize = 512;
+
+  char mBuffer[kBufferSize];
+  size_t mPos;
   NativeFileDesc mFD;
 };
 
 class JSONAnnotationWriter : public AnnotationWriter {
  public:
   explicit JSONAnnotationWriter(PlatformWriter& aPlatformWriter)
       : mWriter(aPlatformWriter), mEmpty(true) {
     mWriter.WriteBuffer("{", 1);
@@ -1512,23 +1539,25 @@ bool MinidumpCallback(
   WriteCrashEventFile(crashTime, crashTimeString, addrInfo,
 #ifdef XP_LINUX
                       descriptor
 #else
                       minidump_id
 #endif
   );
 
-  PlatformWriter apiData;
+  {
+    PlatformWriter apiData;
 #ifdef XP_LINUX
-  OpenAPIData(apiData, descriptor.path());
+    OpenAPIData(apiData, descriptor.path());
 #else
-  OpenAPIData(apiData, dump_path, minidump_id);
+    OpenAPIData(apiData, dump_path, minidump_id);
 #endif
-  WriteAnnotationsForMainProcessCrash(apiData, addrInfo, crashTime);
+    WriteAnnotationsForMainProcessCrash(apiData, addrInfo, crashTime);
+  }
 
   if (!doReport) {
 #ifdef XP_WIN
     TerminateProcess(GetCurrentProcess(), 1);
 #endif  // XP_WIN
     return returnValue;
   }
 
@@ -3144,17 +3173,17 @@ static void ReadAndValidateExceptionTime
       value.Append(c);
     } while (len > 0);
 
     // Looks good, save the (annotation, value) pair
     aAnnotations[static_cast<Annotation>(rawAnnotation)] = value;
   } while (res > 0);
 }
 
-static bool WriteExtraFile(PlatformWriter pw,
+static bool WriteExtraFile(PlatformWriter& pw,
                            const AnnotationTable& aAnnotations) {
   if (!pw.Valid()) {
     return false;
   }
 
   JSONAnnotationWriter writer(pw);
   for (auto key : MakeEnumeratedRange(Annotation::Count)) {
     const nsCString& value = aAnnotations[key];
@@ -3176,17 +3205,18 @@ bool WriteExtraFile(const nsAString& id,
 #ifdef XP_WIN
   nsAutoString path;
   NS_ENSURE_SUCCESS(extra->GetPath(path), false);
 #elif defined(XP_UNIX)
   nsAutoCString path;
   NS_ENSURE_SUCCESS(extra->GetNativePath(path), false);
 #endif
 
-  return WriteExtraFile(PlatformWriter(path.get()), annotations);
+  PlatformWriter pw(path.get());
+  return WriteExtraFile(pw, annotations);
 }
 
 static void ReadExceptionTimeAnnotations(AnnotationTable& aAnnotations,
                                          uint32_t aPid) {
   // Read exception-time annotations
   StaticMutexAutoLock pidMapLock(processMapLock);
   if (aPid && processToCrashFd.count(aPid)) {
     PRFileDesc* prFd = processToCrashFd[aPid];