artox/incubator-pagespeed-ngx
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Preload hinting: order nested stylesheets after their parent,

Browse Source 
before later resources.
This commit is contained in:
Maks Orlovich
2016-09-08 12:28:54 -04:00
parent 1a0eb2217c
commit 8d585f7508
8 changed files with 262 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
net/instaweb/rewriter/collect_dependencies_filter.cc
+20 -5
View File
@@ -213,15 +213,30 @@ class CollectDependenciesFilter::Context : public RewriteContext {
output_partition(0)->collected_dependency_size() > 0) {
CachedResult* result = output_partition(0);
// Top-level stuff just gets its dep_id_ as the sorting key.
result->mutable_collected_dependency(0)->add_order_key(dep_id_);
dep_tracker->ReportDependencyCandidate(dep_id_,
&result->collected_dependency(0));
// Any other dependencies get their brand new IDs.
// TODO(morlovich): They actually should be hinted right after dependency
// dep_id_.
// Any other dependencies stored in result->collected_dependency >= 1
// are things we discovered *inside* whatever is described by
// result->collected_dependency(0)
//
// We grab a brand new ID for each one's storage inside
// dependency_tracker, and give them sorting keys based on the parent's
// dep_id_: (dep_id_, 1), (dep_id_, 2), etc., and so on, to make them get
// sorted after their parent (whose sorting key will be (dep_id_)) and
// before the next top-level resource, which will be something like
// (dep_id_ + 1) or some larger number. Note that we produce order keys
// at most 2 deep because we (for now?) only collect dependencies that
// deep.
for (int c = 1; c < result->collected_dependency_size(); ++c) {
int additional_dep_id = dep_tracker->RegisterDependencyCandidate();
dep_tracker->ReportDependencyCandidate(
additional_dep_id, &result->collected_dependency(c));
Dependency* child_dep = result->mutable_collected_dependency(c);
child_dep->add_order_key(dep_id_);
child_dep->add_order_key(c);
dep_tracker->ReportDependencyCandidate(additional_dep_id, child_dep);
}
} else {
dep_tracker->ReportDependencyCandidate(dep_id_, nullptr);
net/instaweb/rewriter/collect_dependencies_filter_test.cc
+87 -1
View File
@@ -16,11 +16,12 @@
// Author: morlovich@google.com (Maksim Orlovich)
#include "net/instaweb/rewriter/public/dependency_tracker.h"
#include "net/instaweb/rewriter/public/collect_dependencies_filter.h"
#include "net/instaweb/http/public/http_cache.h"
#include "net/instaweb/http/public/http_cache_failure.h"
#include "net/instaweb/rewriter/dependencies.pb.h"
#include "net/instaweb/rewriter/public/dependency_tracker.h"
#include "net/instaweb/rewriter/public/rewrite_driver.h"
#include "net/instaweb/rewriter/public/rewrite_options.h"
#include "net/instaweb/rewriter/public/rewrite_test_base.h"
@@ -126,6 +127,7 @@ TEST_F(CollectDependenciesFilterTest, BasicOperation) {
"expiration_time_ms: ", FormatRelTimeSec(kYearSec), " "
"date_ms: ", FormatRelTimeSec(0),
"}"
"order_key: 0"
"}"
"dependency {"
"url: 'http://test.com/b.js.pagespeed.jm.0.js'"
@@ -141,6 +143,7 @@ TEST_F(CollectDependenciesFilterTest, BasicOperation) {
"expiration_time_ms: ", FormatRelTimeSec(kYearSec), " "
"date_ms: ", FormatRelTimeSec(0),
"}"
"order_key: 1"
"}")));
rewrite_driver()->FinishParse();
@@ -173,6 +176,7 @@ TEST_F(CollectDependenciesFilterTest, MediaTopLevel) {
"expiration_time_ms: ", FormatRelTimeSec(100), " "
"date_ms: ", FormatRelTimeSec(0),
"}"
"order_key: 0"
"}"
"dependency {"
"url: 'http://test.com/e.css'"
@@ -182,6 +186,7 @@ TEST_F(CollectDependenciesFilterTest, MediaTopLevel) {
"expiration_time_ms: ", FormatRelTimeSec(400), " "
"date_ms: ", FormatRelTimeSec(0),
"}"
"order_key: 1"
"}")));
rewrite_driver()->FinishParse();
@@ -214,10 +219,12 @@ TEST_F(CollectDependenciesFilterTest, HandleEmptyResources) {
"dependency {"
"url: 'http://test.com/e.css'"
"content_type: DEP_CSS "
"order_key: 0"
"}"
"dependency {"
"url: 'http://test.com/f.js'"
"content_type: DEP_JAVASCRIPT "
"order_key: 1"
"}"));
rewrite_driver()->FinishParse();
@@ -242,6 +249,7 @@ TEST_F(CollectDependenciesFilterTest, Unoptimized) {
"expiration_time_ms: ", FormatRelTimeSec(100), " "
"date_ms: ", FormatRelTimeSec(0),
"}"
"order_key: 0"
"}"
"dependency {"
"url: 'http://test.com/b.js'"
@@ -251,6 +259,7 @@ TEST_F(CollectDependenciesFilterTest, Unoptimized) {
"expiration_time_ms: ", FormatRelTimeSec(200), " "
"date_ms: ", FormatRelTimeSec(0),
"}"
"order_key: 1"
"}")));
rewrite_driver()->FinishParse();
@@ -329,6 +338,7 @@ TEST_F(CollectDependenciesFilterTest, Combiners) {
"expiration_time_ms: ", FormatRelTimeSec(kYearSec), " "
"date_ms: ", FormatRelTimeSec(0),
"}"
"order_key: 0"
"}")));
rewrite_driver()->FinishParse();
}
@@ -384,6 +394,7 @@ TEST_F(CollectDependenciesFilterTest, Chain) {
"expiration_time_ms: ", FormatRelTimeSec(kYearSec), " "
"date_ms: ", FormatRelTimeSec(0),
"}"
"order_key: 0"
"}"),
"dependency {"
"url: 'http://test.com/b.js.pagespeed.jm.0.js'"
@@ -399,6 +410,7 @@ TEST_F(CollectDependenciesFilterTest, Chain) {
"expiration_time_ms: ", FormatRelTimeSec(kYearSec), " "
"date_ms: ", FormatRelTimeSec(0),
"}"
"order_key: 2"
"}"
"dependency {"
"url: 'http://test.com/d.js.pagespeed.jm.0.js'"
@@ -414,6 +426,80 @@ TEST_F(CollectDependenciesFilterTest, Chain) {
"expiration_time_ms: ", FormatRelTimeSec(kYearSec), " "
"date_ms: ", FormatRelTimeSec(0),
"}"
"order_key: 3"
"}")));
rewrite_driver()->FinishParse();
}
TEST_F(CollectDependenciesFilterTest, Indirect) {
// Test that we collect indirect dependencies.
SetResponseWithDefaultHeaders("d.css", kContentTypeCss,
"@import \"i1.css\" all;\n"
"@import \"i2.css\" print, screen;\n"
"@import \"i3.css\" print; ", 300);
options()->EnableFilter(RewriteOptions::kRewriteCss);
rewrite_driver()->AddFilters();
const char kInput[] = "<link rel=stylesheet href=d.css>";
const char kOutput[] =
"<link rel=stylesheet href=A.d.css.pagespeed.cf.0.css>";
ValidateExpected("basic_res", kInput, kOutput);
ResetDriver();
DependencyTracker* tracker = rewrite_driver()->dependency_tracker();
rewrite_driver()->StartParse(kTestDomain);
ASSERT_TRUE(tracker->read_in_info() != nullptr);
EXPECT_THAT(*tracker->read_in_info(), EqualsProto(StrCat(StrCat(
"dependency {"
"url: 'http://test.com/A.d.css.pagespeed.cf.0.css'"
"content_type: DEP_CSS "
"validity_info {"
"type: CACHED "
"expiration_time_ms: ", FormatRelTimeSec(300), " " // d.css
"date_ms: ", FormatRelTimeSec(0),
"}"
"validity_info {"
"type: CACHED "
"last_modified_time_ms: ", FormatRelTimeSec(0), " ", // d.cf
"expiration_time_ms: ", FormatRelTimeSec(kYearSec), " "
"date_ms: ", FormatRelTimeSec(0),
"}"
"order_key: 0"
"}"),
"dependency {"
"url: 'http://test.com/i1.css'"
"content_type: DEP_CSS "
"validity_info {"
"type: CACHED "
"expiration_time_ms: ", FormatRelTimeSec(300), " " // d.css
"date_ms: ", FormatRelTimeSec(0),
"}"
"validity_info {"
"type: CACHED "
"last_modified_time_ms: ", FormatRelTimeSec(0), " ", // d.cf
"expiration_time_ms: ", FormatRelTimeSec(kYearSec), " "
"date_ms: ", FormatRelTimeSec(0),
"}"
"order_key: 0 "
"order_key: 1"
"}"
"dependency {"
"url: 'http://test.com/i2.css'"
"content_type: DEP_CSS "
"validity_info {"
"type: CACHED "
"expiration_time_ms: ", FormatRelTimeSec(300), " " // d.css
"date_ms: ", FormatRelTimeSec(0),
"}"
"validity_info {"
"type: CACHED "
"last_modified_time_ms: ", FormatRelTimeSec(0), " ", // d.cf
"expiration_time_ms: ", FormatRelTimeSec(kYearSec), " "
"date_ms: ", FormatRelTimeSec(0),
"}"
"order_key: 0 "
"order_key: 2"
"}")));
rewrite_driver()->FinishParse();
}
net/instaweb/rewriter/dependencies.proto
+4
View File
@@ -43,6 +43,10 @@ message Dependency {
// including other_dependency, so at the time we have to decide whether to
// push/preload we can tell if .pagespeed. resource has expired yet or not.
repeated InputInfo validity_info = 5;
// Encodes the order of this compared to other dependencies. The semantics
// are lexicographic, e.g. (4) is before (4,1) is before (5).
repeated int32 order_key = 6;
}
// All the candidates for pushing or preloading.
net/instaweb/rewriter/dependency_tracker.cc
+30 -1
View File
@@ -119,5 +119,34 @@ void DependencyTracker::WriteToPropertyCacheIfDone() {
ClearLockHeld();
}
} // namespace net_instaweb
bool DependencyOrderCompator::operator()(
const Dependency& a, const Dependency& b) {
int pos = 0;
while (pos < a.order_key_size() && pos < b.order_key_size()) {
if (a.order_key(pos) < b.order_key(pos)) {
return true;
}
if (a.order_key(pos) > b.order_key(pos)) {
return false;
}
++pos;
}
if (pos == a.order_key_size()) {
// a at end.
if (pos == b.order_key_size()) {
// b also at end -> they're the same.
return false;
} else {
// not at end -> a is prefix of b, so a < b
return true;
}
} else {
// a not at end, b at end => b is a prefix of a, so b < a, so
// clearly not a < b
return false;
}
}
} // namespace net_instaweb
net/instaweb/rewriter/dependency_tracker_test.cc
+85
View File
@@ -197,6 +197,91 @@ TEST_F(DependencyTrackerTest, ViaRewriteDriverOff) {
TestInRewriteDriver(false);
}
TEST_F(DependencyTrackerTest, Comparator) {
Dependency de;
Dependency de1;
de1.add_order_key(1);
Dependency de12;
de12.add_order_key(1);
de12.add_order_key(2);
Dependency de121;
de121.add_order_key(1);
de121.add_order_key(2);
de121.add_order_key(1);
Dependency de122;
de122.add_order_key(1);
de122.add_order_key(2);
de122.add_order_key(2);
Dependency de13;
de13.add_order_key(1);
de13.add_order_key(3);
Dependency de2;
de2.add_order_key(2);
DependencyOrderCompator less;
EXPECT_FALSE(less(de, de));
EXPECT_TRUE(less(de, de1));
EXPECT_TRUE(less(de, de12));
EXPECT_TRUE(less(de, de121));
EXPECT_TRUE(less(de, de122));
EXPECT_TRUE(less(de, de13));
EXPECT_TRUE(less(de, de2));
EXPECT_FALSE(less(de1, de));
EXPECT_FALSE(less(de1, de1));
EXPECT_TRUE(less(de1, de12));
EXPECT_TRUE(less(de1, de121));
EXPECT_TRUE(less(de1, de122));
EXPECT_TRUE(less(de1, de13));
EXPECT_TRUE(less(de1, de2));
EXPECT_FALSE(less(de12, de));
EXPECT_FALSE(less(de12, de1));
EXPECT_FALSE(less(de12, de12));
EXPECT_TRUE(less(de12, de121));
EXPECT_TRUE(less(de12, de122));
EXPECT_TRUE(less(de12, de13));
EXPECT_TRUE(less(de12, de2));
EXPECT_FALSE(less(de121, de));
EXPECT_FALSE(less(de121, de1));
EXPECT_FALSE(less(de121, de12));
EXPECT_FALSE(less(de121, de121));
EXPECT_TRUE(less(de121, de122));
EXPECT_TRUE(less(de121, de13));
EXPECT_TRUE(less(de121, de2));
EXPECT_FALSE(less(de122, de));
EXPECT_FALSE(less(de122, de1));
EXPECT_FALSE(less(de122, de12));
EXPECT_FALSE(less(de122, de121));
EXPECT_FALSE(less(de122, de122));
EXPECT_TRUE(less(de122, de13));
EXPECT_TRUE(less(de122, de2));
EXPECT_FALSE(less(de13, de));
EXPECT_FALSE(less(de13, de1));
EXPECT_FALSE(less(de13, de12));
EXPECT_FALSE(less(de13, de121));
EXPECT_FALSE(less(de13, de122));
EXPECT_FALSE(less(de13, de13));
EXPECT_TRUE(less(de13, de2));
EXPECT_FALSE(less(de2, de));
EXPECT_FALSE(less(de2, de1));
EXPECT_FALSE(less(de2, de12));
EXPECT_FALSE(less(de2, de121));
EXPECT_FALSE(less(de2, de122));
EXPECT_FALSE(less(de2, de13));
EXPECT_FALSE(less(de2, de2));
}
} // namespace
} // namespace net_instaweb
net/instaweb/rewriter/public/dependency_tracker.h
+7
View File
@@ -102,6 +102,13 @@ class DependencyTracker {
DISALLOW_COPY_AND_ASSIGN(DependencyTracker);
};
// Compares two Dependency objects based on the order_key field.
class DependencyOrderCompator {
public:
bool operator()(const Dependency& a, const Dependency& b);
};
} // namespace net_instaweb
#endif // NET_INSTAWEB_REWRITER_PUBLIC_DEPENDENCY_TRACKER_H_
net/instaweb/rewriter/push_preload_filter.cc
+11 -2
View File
@@ -25,11 +25,13 @@
#include "net/instaweb/rewriter/public/push_preload_filter.h"
#include <algorithm>
#include <unordered_set>
#include <utility> // for pair
#include "base/logging.h"
#include "net/instaweb/rewriter/dependencies.pb.h"
#include "net/instaweb/rewriter/public/collect_dependencies_filter.h"
#include "net/instaweb/rewriter/public/dependency_tracker.h"
#include "net/instaweb/rewriter/public/rewrite_driver.h"
#include "pagespeed/kernel/base/string.h"
@@ -58,10 +60,17 @@ void PushPreloadFilter::StartDocumentImpl() {
const Dependencies* deps = driver()->dependency_tracker()->read_in_info();
CHECK(deps != nullptr) << "DetermineEnabled should have prevented this";
// Sort dependencies by order_key.
std::vector<Dependency> ordered_deps;
for (int i = 0, n = deps->dependency_size(); i < n; ++i) {
ordered_deps.push_back(deps->dependency(i));
}
DependencyOrderCompator dep_order;
std::sort(ordered_deps.begin(), ordered_deps.end(), dep_order);
std::unordered_set<GoogleString> already_seen;
for (int i = 0, n = deps->dependency_size(); i < n; ++i) {\
const Dependency& dep = deps->dependency(i);
for (const Dependency& dep : ordered_deps) {
GoogleUrl dep_url(dep.url());
if (!dep_url.IsWebValid()) {
net/instaweb/rewriter/push_preload_filter_test.cc
+16 -4
View File
@@ -115,17 +115,21 @@ TEST_F(PushPreloadFilterTest, BasicOperation) {
}
TEST_F(PushPreloadFilterTest, IndirectCollected) {
// Cover indirect dependencies collected by css_filter here, too.
SetResponseWithDefaultHeaders("c.css", kContentTypeCss,
"@import \"i1.css\" all;\n"
"@import \"i2.css\" print, screen;\n"
"@import \"i3.css\" print; ", 100);
SetResponseWithDefaultHeaders("d.css", kContentTypeCss,
"@import \"i1.css\" all; \n"
"@import \"i4.css\"; ", 100);
options()->EnableFilter(RewriteOptions::kRewriteCss);
rewrite_driver()->AddFilters();
const char kInput[] = "<link rel=stylesheet href=c.css>";
const char kInput[] = "<link rel=stylesheet href=c.css>"
"<link rel=stylesheet href=d.css>";
const char kOutput[] =
"<link rel=stylesheet href=A.c.css.pagespeed.cf.0.css>";
"<link rel=stylesheet href=A.c.css.pagespeed.cf.0.css>"
"<link rel=stylesheet href=A.d.css.pagespeed.cf.0.css>";
ValidateExpected("basic_res", kInput, kOutput);
@@ -140,14 +144,22 @@ TEST_F(PushPreloadFilterTest, IndirectCollected) {
HttpAttributes::kLink, &links));
rewrite_driver()->FinishParse();
ASSERT_EQ(3, links.size());
ASSERT_EQ(5, links.size());
// These should be in preorder wrt to the dependencies between
// CSS and things in it
EXPECT_STREQ("</A.c.css.pagespeed.cf.0.css>; rel=\"preload\"; as=style",
*links[0]);
EXPECT_STREQ("</i1.css>; rel=\"preload\"; as=style",
*links[1]);
EXPECT_STREQ("</i2.css>; rel=\"preload\"; as=style",
*links[2]);
EXPECT_STREQ("</A.d.css.pagespeed.cf.0.css>; rel=\"preload\"; as=style",
*links[3]);
// not i3, since it's print only.
// i1 already hinted.
// i4 isn't, though.
EXPECT_STREQ("</i4.css>; rel=\"preload\"; as=style", *links[4]);
}
} // namespace