Bug 1480146 - Add a cache for inline stylesheets without @import rules. r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 16 Aug 2019 10:56:23 +0000
changeset 488491 f47314ec33ff5b6f1eaf95ca1a721bcf34ce5554
parent 488490 852c5aaa34f7a83dbad736b459b80acb20838112
child 488492 ff3ba7a16c214c01e270483ba42dc37e5bcd22f6
push id36444
push userccoroiu@mozilla.com
push dateFri, 16 Aug 2019 16:24:18 +0000
treeherdermozilla-central@8a9e9189cd98 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1480146, 246490
milestone70.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 1480146 - Add a cache for inline stylesheets without @import rules. r=heycam Supporting @import was much more annoying (I had a patch but it became much more complex than this), and seems other browsers don't do it either: https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/css/style_sheet_contents.cc?l=149&rcl=1999822baf4dc673088e65997fa07ad158f2e166 https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/css/StyleSheetContents.cpp?rev=246490#L111 Also added a singleton perf test for <link> caching. Maybe we should add some cache eviction here, but I'm not sure what'd be the best policy here. Differential Revision: https://phabricator.services.mozilla.com/D41564
layout/style/Loader.cpp
testing/talos/talos/tests/perf-reftest-singletons/inline-style-cache-1.html
testing/talos/talos/tests/perf-reftest-singletons/link-style-cache-1.html
testing/talos/talos/tests/perf-reftest-singletons/perf_reftest_singletons.manifest
--- a/layout/style/Loader.cpp
+++ b/layout/style/Loader.cpp
@@ -456,21 +456,40 @@ bool LoaderReusableStyleSheets::FindReus
 struct Loader::Sheets {
   nsRefPtrHashtable<SheetLoadDataHashKey, StyleSheet> mCompleteSheets;
 
   nsRefPtrHashtable<SheetLoadDataHashKey, SheetLoadData> mPendingDatas;
 
   // The SheetLoadData pointers in mLoadingDatas below are weak references.
   nsDataHashtable<SheetLoadDataHashKey, SheetLoadData*> mLoadingDatas;
 
+  nsRefPtrHashtable<nsStringHashKey, StyleSheet> mInlineSheets;
+
+
+  RefPtr<StyleSheet> LookupInline(const nsAString&);
+
   // A cache hit or miss. It is a miss if the `StyleSheet` is null.
   using CacheResult = Tuple<RefPtr<StyleSheet>, SheetState>;
   CacheResult Lookup(SheetLoadDataHashKey&, bool aSyncLoad);
 };
 
+RefPtr<StyleSheet> Loader::Sheets::LookupInline(const nsAString& aBuffer) {
+  auto result = mInlineSheets.Lookup(aBuffer);
+  if (!result) {
+    return nullptr;
+  }
+  if (result.Data()->HasForcedUniqueInner()) {
+    // Remove it now that we know that we're never going to use this stylesheet
+    // again.
+    result.Remove();
+    return nullptr;
+  }
+  return result.Data()->Clone(nullptr, nullptr, nullptr, nullptr);
+}
+
 static void AssertComplete(const StyleSheet& aSheet) {
   // This sheet came from the XUL cache or our per-document hashtable; it
   // better be a complete sheet.
   MOZ_ASSERT(aSheet.IsComplete(),
              "Sheet thinks it's not complete while we think it is");
 }
 
 static void AssertIncompleteSheetMatches(const SheetLoadData& aData,
@@ -1820,66 +1839,90 @@ Result<Loader::LoadSheetResult, nsresult
       do_QueryInterface(aInfo.mContent));
   NS_ASSERTION(owningElement, "Element is not a style linking element!");
 
   // Since we're not planning to load a URI, no need to hand a principal to the
   // load data or to CreateSheet().
 
   // Check IsAlternateSheet now, since it can mutate our document.
   auto isAlternate = IsAlternateSheet(aInfo.mTitle, aInfo.mHasAlternateRel);
+  LOG(("  Sheet is alternate: %d", static_cast<int>(isAlternate)));
 
   // Use the document's base URL so that @import in the inline sheet picks up
   // the right base.
   nsIURI* baseURI = aInfo.mContent->GetBaseURI();
   nsIURI* sheetURI = aInfo.mContent->OwnerDoc()->GetDocumentURI();
   nsIURI* originalURI = nullptr;
 
   MOZ_ASSERT(aInfo.mIntegrity.IsEmpty());
-  auto sheet = MakeRefPtr<StyleSheet>(eAuthorSheetFeatures, aInfo.mCORSMode,
-                                      SRIMetadata{});
-  sheet->SetURIs(sheetURI, originalURI, baseURI);
-  nsCOMPtr<nsIReferrerInfo> referrerInfo =
-      ReferrerInfo::CreateForInternalCSSResources(aInfo.mContent->OwnerDoc());
-  sheet->SetReferrerInfo(referrerInfo);
 
-  nsIPrincipal* principal = aInfo.mContent->NodePrincipal();
-  if (aInfo.mTriggeringPrincipal) {
-    // The triggering principal may be an expanded principal, which is safe to
-    // use for URL security checks, but not as the loader principal for a
-    // stylesheet. So treat this as principal inheritance, and downgrade if
-    // necessary.
-    principal =
-        BasePrincipal::Cast(aInfo.mTriggeringPrincipal)->PrincipalToInherit();
+  // We only cache sheets if in shadow trees, since regular document sheets are
+  // likely to be unique.
+  const bool isWorthCaching = aInfo.mContent->IsInShadowTree();
+  RefPtr<StyleSheet> sheet;
+  if (isWorthCaching) {
+    if (!mSheets) {
+      mSheets = MakeUnique<Sheets>();
+    }
+    sheet = mSheets->LookupInline(aBuffer);
   }
+  const bool sheetFromCache = !!sheet;
+  if (!sheet) {
+    sheet = MakeRefPtr<StyleSheet>(eAuthorSheetFeatures, aInfo.mCORSMode,
+                                   SRIMetadata{});
+    sheet->SetURIs(sheetURI, originalURI, baseURI);
+    nsCOMPtr<nsIReferrerInfo> referrerInfo =
+        ReferrerInfo::CreateForInternalCSSResources(aInfo.mContent->OwnerDoc());
+    sheet->SetReferrerInfo(referrerInfo);
 
-  // We never actually load this, so just set its principal directly
-  sheet->SetPrincipal(principal);
+    nsIPrincipal* principal = aInfo.mContent->NodePrincipal();
+    if (aInfo.mTriggeringPrincipal) {
+      // The triggering principal may be an expanded principal, which is safe to
+      // use for URL security checks, but not as the loader principal for a
+      // stylesheet. So treat this as principal inheritance, and downgrade if
+      // necessary.
+      principal =
+          BasePrincipal::Cast(aInfo.mTriggeringPrincipal)->PrincipalToInherit();
+    }
 
-  LOG(("  Sheet is alternate: %d", static_cast<int>(isAlternate)));
+    // We never actually load this, so just set its principal directly
+    sheet->SetPrincipal(principal);
+  }
 
   auto matched = PrepareSheet(*sheet, aInfo.mTitle, aInfo.mMedia, nullptr,
                               isAlternate, aInfo.mIsExplicitlyEnabled);
 
   InsertSheetInTree(*sheet, aInfo.mContent);
 
-  auto data = MakeRefPtr<SheetLoadData>(
-      this, aInfo.mTitle, nullptr, sheet, false, owningElement, isAlternate,
-      matched, aObserver, nullptr, aInfo.mReferrerInfo, aInfo.mContent);
-  data->mLineNumber = aLineNumber;
-  // Parse completion releases the load data.
-  //
-  // Note that we need to parse synchronously, since the web expects that the
-  // effects of inline stylesheets are visible immediately (aside from
-  // @imports).
-  NS_ConvertUTF16toUTF8 utf8(aBuffer);
-  Completed completed = ParseSheet(utf8, *data, AllowAsyncParse::No);
+  Completed completed;
+  if (sheetFromCache) {
+    MOZ_ASSERT(sheet->IsComplete());
+    completed = Completed::Yes;
+  } else {
+    auto data = MakeRefPtr<SheetLoadData>(
+        this, aInfo.mTitle, nullptr, sheet, false, owningElement, isAlternate,
+        matched, aObserver, nullptr, aInfo.mReferrerInfo, aInfo.mContent);
+    data->mLineNumber = aLineNumber;
+    // Parse completion releases the load data.
+    //
+    // Note that we need to parse synchronously, since the web expects that the
+    // effects of inline stylesheets are visible immediately (aside from
+    // @imports).
+    NS_ConvertUTF16toUTF8 utf8(aBuffer);
+    completed = ParseSheet(utf8, *data, AllowAsyncParse::No);
+    if (completed == Completed::Yes) {
+      // TODO(emilio): Try to cache sheets with @import rules, maybe?
+      if (isWorthCaching) {
+        mSheets->mInlineSheets.Put(aBuffer, sheet);
+      }
+    } else {
+      data->mMustNotify = true;
+    }
+  }
 
-  if (completed == Completed::No) {
-    data->mMustNotify = true;
-  }
   return LoadSheetResult{completed, isAlternate, matched};
 }
 
 Result<Loader::LoadSheetResult, nsresult> Loader::LoadStyleLink(
     const SheetInfo& aInfo, nsICSSLoaderObserver* aObserver) {
   MOZ_ASSERT(aInfo.mURI, "Must have URL to load");
   LOG(("css::Loader::LoadStyleLink"));
   LOG_URI("  Link uri: '%s'", aInfo.mURI);
new file mode 100644
--- /dev/null
+++ b/testing/talos/talos/tests/perf-reftest-singletons/inline-style-cache-1.html
@@ -0,0 +1,24 @@
+<!doctype html>
+<script src="util.js"></script>
+<body>
+<script>
+const styles = `:host { background: red; width: 100px; height: 100px; display: block; }`.repeat(10000);
+customElements.define("custom-element", class CustomElement extends HTMLElement {
+  connectedCallback() {
+    this.attachShadow({ mode: "open" });
+    const style = document.createElement("style");
+    style.textContent = styles;
+    this.shadowRoot.appendChild(style);
+  }
+});
+
+perf_start();
+
+for (let i = 0; i < 1000; ++i)
+  document.body.appendChild(document.createElement("custom-element"));
+
+onload = function() {
+  flush_layout(document.body);
+  perf_finish();
+};
+</script>
new file mode 100644
--- /dev/null
+++ b/testing/talos/talos/tests/perf-reftest-singletons/link-style-cache-1.html
@@ -0,0 +1,26 @@
+<!doctype html>
+<script src="util.js"></script>
+<body>
+<script>
+const styles = `:host { background: red; width: 100px; height: 100px; display: block; }`.repeat(10000);
+const blob = URL.createObjectURL(new Blob([styles], { type: 'text/css' }));
+customElements.define("custom-element", class CustomElement extends HTMLElement {
+  connectedCallback() {
+    this.attachShadow({ mode: "open" });
+    const link = document.createElement("link");
+    link.rel = "stylesheet";
+    link.href = blob;
+    this.shadowRoot.appendChild(link);
+  }
+});
+
+perf_start();
+
+for (let i = 0; i < 1000; ++i)
+  document.body.appendChild(document.createElement("custom-element"));
+
+onload = function() {
+  flush_layout(document.body);
+  perf_finish();
+};
+</script>
--- a/testing/talos/talos/tests/perf-reftest-singletons/perf_reftest_singletons.manifest
+++ b/testing/talos/talos/tests/perf-reftest-singletons/perf_reftest_singletons.manifest
@@ -18,8 +18,10 @@
 % http://localhost/tests/perf-reftest-singletons/id-getter-2.html
 % http://localhost/tests/perf-reftest-singletons/id-getter-3.html
 % http://localhost/tests/perf-reftest-singletons/id-getter-4.html
 % http://localhost/tests/perf-reftest-singletons/id-getter-5.html
 % http://localhost/tests/perf-reftest-singletons/id-getter-6.html
 % http://localhost/tests/perf-reftest-singletons/id-getter-7.html
 % http://localhost/tests/perf-reftest-singletons/abspos-reflow-1.html
 % http://localhost/tests/perf-reftest-singletons/scrollbar-styles-1.html
+% http://localhost/tests/perf-reftest-singletons/inline-style-cache-1.html
+% http://localhost/tests/perf-reftest-singletons/link-style-cache-1.html