Skip to content

Commit

Permalink
Front load and cache some page chain class prep
Browse files Browse the repository at this point in the history
This takes some work that used to happen for each request and caches it.

This is mostly groundwork for fine-grained instrumentation of page lifecycle
methods.  It incidentally also slightly reduces the overhead of building up
the page chain object during each request.

[core/__bench__/handlePage.js](redfin#926) before:
```
1       0.000481308279135711
10      0.0009405891195623999
100     0.005067864883424908
1000    0.04754033069230767
```

[core/__bench__/handlePage.js](redfin#926) after:
```
1       0.0004272630582241284
10      0.0006927164659505115
100     0.0034683026867557273
1000    0.031324801985714294
```

Basically reduces middleware overhead to from ~50µs to ~30µs.  Not a huge
deal, but the instrumentation is likely to add a little overhead, so it's nice
to pay some of that down ahead of time. 😁
  • Loading branch information
gigabo committed Jun 20, 2017
1 parent 67d31ae commit 54c75ad
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 64 deletions.
6 changes: 3 additions & 3 deletions packages/react-server/core/__tests__/reactMiddlewareSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe("renderMiddleware", () => {

describe("null values", () => {
beforeAll(() => {
page = PageUtil.createPageChain([new NullValuesPage()]);
page = PageUtil.createPageChain(NullValuesPage);
});

beforeEach(() => {
Expand Down Expand Up @@ -51,7 +51,7 @@ describe("renderMiddleware", () => {

describe("promises with null values", () => {
beforeAll(() => {
page = PageUtil.createPageChain([new NullValuePromisesPage()]);
page = PageUtil.createPageChain(NullValuePromisesPage);
});

beforeEach(() => {
Expand Down Expand Up @@ -89,7 +89,7 @@ describe("renderMiddleware", () => {

describe("good values", () => {
beforeAll(() => {
page = PageUtil.createPageChain([new NormalValuesPage()]);
page = PageUtil.createPageChain(NormalValuesPage);
});

beforeEach(() => {
Expand Down
27 changes: 1 addition & 26 deletions packages/react-server/core/context/Navigator.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,19 +159,7 @@ class Navigator extends EventEmitter {
}

handlePage(pageConstructor, request, type) {
// instantiate the pages we need to fulfill this request.
var pageClasses = [];

this._addPageMiddlewareToArray(this._globalMiddleware, pageClasses);
this._addPageMiddlewareToArray([pageConstructor], pageClasses);

var pages = pageClasses.map((pageClass) => {
if (Object.getOwnPropertyNames(pageClass).length === 0) {
throw new Error("Tried to instantiate a page or middleware class that was an empty object. Did you forget to assign a class to module.exports?");
}
return new pageClass();
});
var page = PageUtil.createPageChain(pages);
var page = PageUtil.createPageChain(pageConstructor, this._globalMiddleware);

this.emit("page", page);

Expand Down Expand Up @@ -223,19 +211,6 @@ class Navigator extends EventEmitter {

}

/**
* recursively adds the middleware in the pages array to array.
*/
_addPageMiddlewareToArray(pages, array) {
if (!pages) return;
pages.forEach((page) => {
if (page.middleware) {
this._addPageMiddlewareToArray(page.middleware(), array);
}
array.push(page);
});
}

getState () {
return {
loading: this._loading,
Expand Down
112 changes: 77 additions & 35 deletions packages/react-server/core/util/PageUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,15 @@ var PAGE_METHODS = {
getResponseData : [() => "", Q],
};

// Apply the standardization functions to each default implementation.
Object.keys(PAGE_METHODS).forEach(method => {
var [defaultImpl, standardize] = PAGE_METHODS[method];
var standardizedDefaultImpl = function() {
return standardize(defaultImpl.apply(this, arguments));
}
PAGE_METHODS[method] = [standardizedDefaultImpl, standardize];
});

// These are similar to `PAGE_METHODS`, but differ as follows:
//
// - They are not chained.
Expand Down Expand Up @@ -216,26 +225,6 @@ function makeSetter(key){
}
}

// This attaches `PAGE_MIXIN` methods to page/middleware classes.
//
// It does this only _once_, and thereafter short-circuits.
//
function lazyMixinPageUtilMethods(page){
var proto = Object.getPrototypeOf(page);
if (proto._haveMixedInPageUtilMethods) return;

proto._haveMixedInPageUtilMethods = true;

Object.keys(PAGE_MIXIN).forEach(method => {
if (proto[method]){
throw new Error(`PAGE_MIXINS method override: ${
(proto.constructor||{}).name
}.${method}`);
}
proto[method] = PAGE_MIXIN[method];
});
}

// These `standardize*` functions show what will happen to the output of your
// page methods.
//
Expand Down Expand Up @@ -318,21 +307,67 @@ function logInvocation(name, func){
}
}

// Return `fn` with a wrapper that puts its return value through `standardize`
// on the way out.
function makeStandard(standardize, fn){
return function(){
return standardize(fn.apply(null, [].slice.call(arguments)));
}
}

function makeArray(valueOrArray) {
if (!Array.isArray(valueOrArray)) {
return [valueOrArray];
}
return valueOrArray;
}

/**
* recursively adds the middleware in the pages array to array.
*/
function addPageMiddlewareToArray(pages, array) {
if (!pages) return;

pages.forEach((page) => {

if (Object.getOwnPropertyNames(page).length === 0) {
throw new Error("Tried to instantiate a page or middleware class that was an empty object. Did you forget to assign a class to module.exports?");
}

if (page.middleware) {
addPageMiddlewareToArray(page.middleware(), array);
}

prepPageForUse(page);

array.push(page);
});
}

function prepPageForUse(page) {

// Hack around es6 classes.
var proto = Object.getPrototypeOf(new page);

// We may have already hit this middleware.
if (!proto._reactServerHasAugmented) {

proto._reactServerHasAugmented = true;

// Mix in the methods that the page will be able to call on itself.
Object.keys(PAGE_MIXIN).forEach(method => {

if (proto[method]) {
throw new Error(`PAGE_MIXINS method override: ${page.name}.${method}`);
}
proto[method] = PAGE_MIXIN[method];
});

Object.keys(PAGE_METHODS).forEach(method => {
var orig = proto[method];
if (orig) {
var standardize = PAGE_METHODS[method][1];

proto[method] = function() {
return standardize(orig.apply(this, arguments));
}
}
});
}
}

var PageUtil = {
PAGE_METHODS,

Expand All @@ -350,7 +385,19 @@ var PageUtil = {
// - PAGE_METHODS
// - PAGE_HOOKS
//
createPageChain(pages) {
createPageChain(pageConstructor, globalMiddleware) {

// instantiate the pages we need to fulfill this request.
var classes = pageConstructor.__rsClasses || (pageConstructor.__rsClasses = []);

if (!classes.length) {

addPageMiddlewareToArray(globalMiddleware, classes);
addPageMiddlewareToArray([pageConstructor], classes);
}

var pages = classes.map(Page => new Page);

/* eslint-disable no-loop-func */

// This will be our return value.
Expand All @@ -360,16 +407,12 @@ var PageUtil = {
//
var pageChain = Object.create(PAGE_CHAIN_PROTOTYPE);

// Make sure all page classes have been augmented with the
// methods provided by `PAGE_MIXIN`.
pages.forEach(lazyMixinPageUtilMethods);

// Wire up the chained methods.
for (var method in PAGE_METHODS){

if (!PAGE_METHODS.hasOwnProperty(method)) continue;

var [defaultImpl, standardize] = PAGE_METHODS[method];
var [defaultImpl] = PAGE_METHODS[method];

// Take bound methods for each page/middleware that
// implements (plus the default implementation), and
Expand All @@ -385,7 +428,6 @@ var PageUtil = {
.filter (page => page[method])
.map (page => page[method].bind(page))
.concat ([defaultImpl])
.map (makeStandard.bind(null, standardize))
.reduceRight ((next, cur) => cur.bind(null, next))
);
}
Expand Down

0 comments on commit 54c75ad

Please sign in to comment.