Skip to content

Commit

Permalink
fix(perfs): split keys computation into smaller functions
Browse files Browse the repository at this point in the history
  • Loading branch information
stafyniaksacha committed Mar 18, 2022
1 parent 357ede2 commit 5aba888
Show file tree
Hide file tree
Showing 11 changed files with 109 additions and 84 deletions.
31 changes: 16 additions & 15 deletions BENCHMARKS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,53 +2,54 @@

## Context

- Rest cache version: `4.2.2`
- Strapi version: `4.1.0`
- Endpoint used: `/api/homepage?populate=*` from `playground/memory`
- Rest cache version: `4.2.4`
- Strapi version: `4.1.5`
- Node version: `16.13.2`
- Endpoint used: `/api/homepage?populate=*` from `shared/api`

## Runs

### Cache disabled (reference)

```sh
$ ENABLE_CACHE=false yarn profile
$ ENABLE_CACHE=false yarn profile:memory
```

| Stat | 2.5% | 50% | 97.5% | 99% | Avg | Stdev | Max |
| ----------- | ------- | ------- | ------- | ------- | ---------- | --------- | ------- |
| **Latency** | 2707 ms | 2834 ms | 3322 ms | 3451 ms | 2851.15 ms | 137.25 ms | 3659 ms |
| **Latency** | 2424 ms | 2555 ms | 2921 ms | 3012 ms | 2565.12 ms | 133.23 | 3401 ms |

| Stat | 1% | 2.5% | 50% | 97.5% | Avg | Stdev | Min |
| ------------- | --- | ---- | ------ | ------- | ------ | ------ | ------ |
| **Req/Sec** | 0 | 0 | 153 | 1000 | 348.89 | 378.71 | 6 |
| **Bytes/Sec** | 0 B | 0 B | 176 kB | 1.15 MB | 401 kB | 436 kB | 6.9 kB |
| **Req/Sec** | 0 | 0 | 386 | 1000 | 383.34 | 382.38 | 39 |
| **Bytes/Sec** | 0 B | 0 B | 453 kB | 1.17 MB | 450 kB | 449 kB | 45.7 kB |

### Cache enabled (without etag)

```sh
$ ENABLE_CACHE=true ENABLE_ETAG=false yarn profile
$ ENABLE_ETAG=false yarn profile:memory
```

| Stat | 2.5% | 50% | 97.5% | 99% | Avg | Stdev | Max |
| ----------- | ------ | ------ | ------ | ------ | --------- | ------- | ------ |
| **Latency** | 118 ms | 127 ms | 188 ms | 201 ms | 132.14 ms | 17.5 ms | 306 ms |
| **Latency** | 113 ms | 116 ms | 166 ms | 175 ms | 120.04 ms | 12.86 ms | 275 ms |

| Stat | 1% | 2.5% | 50% | 97.5% | Avg | Stdev | Min |
| ------------- | ------- | ------- | ------- | ------- | ------- | ------ | ------- |
| **Req/Sec** | 5663 | 6211 | 7591 | 8231 | 7536.72 | 518.32 | 4790 |
| **Bytes/Sec** | 6.59 MB | 7.23 MB | 8.84 MB | 9.58 MB | 8.77 MB | 603 kB | 5.58 MB |
| **Req/Sec** | 7451 | 7523 | 8287 | 8687 | 8293.49 | 306.35 | 6381 |
| **Bytes/Sec** | 8.85 MB | 8.93 MB | 9.84 MB | 10.3 MB | 9.84 MB | 364 kB | 7.57 MB |

### Cache enabled (with etag)

```sh
$ ENABLE_CACHE=true ENABLE_ETAG=true yarn profile
$ yarn profile:memory
```

| Stat | 2.5% | 50% | 97.5% | 99% | Avg | Stdev | Max |
| ----------- | ------ | ------ | ------ | ------ | --------- | -------- | ------ |
| **Latency** | 123 ms | 131 ms | 190 ms | 207 ms | 137.06 ms | 17.42 ms | 302 ms |
| **Latency** | 119 ms | 125 ms | 185 ms | 197 ms | 131.05 ms | 16.87 ms | 307 ms |

| Stat | 1% | 2.5% | 50% | 97.5% | Avg | Stdev | Min |
| ------------- | ------- | ------- | ------- | ------- | ------- | ------ | ------- |
| **Req/Sec** | 5599 | 5747 | 7311 | 7923 | 7267.47 | 476.6 | 5591 |
| **Bytes/Sec** | 6.75 MB | 6.93 MB | 8.82 MB | 9.55 MB | 8.76 MB | 575 kB | 6.74 MB |
| **Req/Sec** | 6551 | 6559 | 7651 | 8231 | 7599.39 | 472.94 | 6100 |
| **Bytes/Sec** | 8.05 MB | 8.07 MB | 9.4 MB | 10.1 MB | 9.34 MB | 581 kB | 7.5 MB |
3 changes: 2 additions & 1 deletion packages/strapi-plugin-rest-cache/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@
"@koa/router": "^10.1.1",
"chalk": "^4.1.2",
"debug": "^4.3.4",
"lodash": "^4.17.15"
"lodash": "^4.17.15",
"ohash": "^0.1.0"
},
"devDependencies": {
"@strapi/strapi": "^4.1.5",
Expand Down
32 changes: 13 additions & 19 deletions packages/strapi-plugin-rest-cache/server/middlewares/recv.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,23 @@ const etagMatch = require('../utils/middlewares/etagMatch');
* @param {{ strapi: import('@strapi/strapi').Strapi }} context
*/
module.exports = (options, { strapi }) => {
const { cacheRouteConfig } = options;

if (!cacheRouteConfig) {
if (!options?.cacheRouteConfig) {
throw new Error(
'REST Cache: unable to initialize recv middleware: options.cacheRouteConfig is required'
);
}

const store = strapi.plugin('rest-cache').service('cacheStore');
const { strategy } = strapi.config.get('plugin.rest-cache');
const { cacheRouteConfig } = options;
const { hitpass, maxAge, keys } = cacheRouteConfig;
const { enableEtag = false, enableXCacheHeaders = false } = strategy;

return async function recv(ctx, next) {
// hash
const cacheKey = `${ctx.request.path}?${generateCacheKey(
cacheRouteConfig,
ctx
)}`;
const cacheKey = generateCacheKey(ctx, keys);

// hitpass check
const lookup = shouldLookup(ctx, cacheRouteConfig);
const lookup = shouldLookup(ctx, hitpass);

// keep track of the etag
let etagCached = null;
Expand Down Expand Up @@ -114,19 +110,17 @@ module.exports = (options, { strapi }) => {
ctx.set('ETag', `"${etag}"`);

// persist etag asynchronously
store
.set(`${cacheKey}_etag`, etag, cacheRouteConfig.maxAge)
.catch(() => {
debug(
`[RECV] GET ${cacheKey} ${chalk.yellow(
'Unable to store ETag in cache'
)}`
);
});
store.set(`${cacheKey}_etag`, etag, maxAge).catch(() => {
debug(
`[RECV] GET ${cacheKey} ${chalk.yellow(
'Unable to store ETag in cache'
)}`
);
});
}

// persist cache asynchronously
store.set(cacheKey, ctx.body, cacheRouteConfig.maxAge).catch(() => {
store.set(cacheKey, ctx.body, maxAge).catch(() => {
debug(
`[RECV] GET ${cacheKey} ${chalk.yellow(
'Unable to store Content in cache'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,52 +1,27 @@
'use strict';

/**
* @typedef {import('koa').Context} Context
*/

/**
* Generates a cache key for the current request
*
* @param {CacheRouteConfig} cacheRouteConfig
* @param {Context} ctx
* @return {string}
*/
function generateCacheKey(cacheRouteConfig, ctx) {
const generateHeadersKey = require('./generateHeadersKey');
const generateQueryParamsKey = require('./generateQueryParamsKey');

function generateCacheKey(
ctx,
keys = {
useQueryParams: false, // @todo: array or boolean => can be optimized
useHeaders: [],
}
) {
let querySuffix = '';
let headersSuffix = '';

if (cacheRouteConfig.keys.useQueryParams !== false) {
let keys = [];

if (cacheRouteConfig.keys.useQueryParams === true) {
keys = Object.keys(ctx.query);
} else if (cacheRouteConfig.keys.useQueryParams.length > 0) {
keys = Object.keys(ctx.query).filter((key) =>
cacheRouteConfig.keys.useQueryParams.includes(key)
);
}

querySuffix = keys
.sort()
.map(
(k) =>
`${k}=${
typeof ctx.query[k] === 'object'
? JSON.stringify(ctx.query[k])
: ctx.query[k]
}`
) // query strings are key sensitive
.join(',');
if (keys.useQueryParams !== false) {
querySuffix = generateQueryParamsKey(ctx, keys.useQueryParams);
}

if (cacheRouteConfig.keys.useHeaders.length > 0) {
headersSuffix = cacheRouteConfig.keys.useHeaders
.filter((k) => ctx.request.header[k] !== undefined)
.map((k) => `${k.toLowerCase()}=${ctx.request.header[k.toLowerCase()]}`) // headers are key insensitive
.join(',');
if (keys.useHeaders.length > 0) {
headersSuffix = generateHeadersKey(ctx, keys.useHeaders);
}

return `${querySuffix}&${headersSuffix}`;
return `${ctx.request.path}?${querySuffix}&${headersSuffix}`;
}

module.exports = generateCacheKey;
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
'use strict';

function generateHeadersKey(ctx, useHeaders = []) {
return useHeaders
.filter((k) => ctx.request.header[k] !== undefined)
.map((k) => `${k.toLowerCase()}=${ctx.request.header[k.toLowerCase()]}`) // headers are key insensitive
.join(',');
}

module.exports = generateHeadersKey;
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
'use strict';

function generateQueryParamsKey(
ctx,
useQueryParams = true // @todo: array or boolean => can be optimized
) {
let keys = [];

if (useQueryParams === true) {
keys = Object.keys(ctx.query);
} else if (useQueryParams.length > 0) {
keys = Object.keys(ctx.query).filter((key) => useQueryParams.includes(key));
}

if (keys.length === 0) {
return '';
}

keys.sort();

return keys
.map(
(k) =>
`${k}=${
typeof ctx.query[k] === 'object'
? JSON.stringify(ctx.query[k])
: ctx.query[k]
}`
) // query strings are key sensitive
.join(',');
}

module.exports = generateQueryParamsKey;
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
'use strict';

function shouldLookup(ctx, cacheRouteConfig) {
const type = cacheRouteConfig.hitpass;
function shouldLookup(
ctx,
hitpass // @todo: function or boolean => can be optimized
) {
const type = typeof hitpass;

if (typeof type === 'function') {
return !cacheRouteConfig.hitpass(ctx);
if (type === 'boolean') {
return !hitpass;
}

if (typeof type === 'boolean') {
return !cacheRouteConfig.hitpass;
if (type === 'function') {
return !hitpass(ctx);
}

return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
/**
* Reject promise after timeout
*
* @todo this is slow, we should find a way to do this in a faster way
* @param {Promise<any>} callback
* @param {number} ms
* @return {Promise<never>}
Expand Down
1 change: 1 addition & 0 deletions playgrounds/memory/config/plugins.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

module.exports = ({ env }) => ({
"rest-cache": {
enabled: env.bool("ENABLE_CACHE", true),
config: {
provider: {
name: "memory",
Expand Down
7 changes: 4 additions & 3 deletions shared/tests/single-type-default.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@ process.env.STRAPI_TELEMETRY_DISABLED = true;

describe.each([
["default settings"],
["empty string keyprefix", { keyprefix: "" }],
["custom keyprefix", { keyprefix: "my-custom-keyprefix" }],
])('single-type with: %s', (testname = '', env = {}) => {
beforeAll(async () => {
if (env.keyprefix) {
process.env.KEYS_PREFIX = undefined;

if (typeof env.keyprefix !== 'undefined') {
process.env.KEYS_PREFIX = env.keyprefix;
} else {
process.env.KEYS_PREFIX = undefined;
}

await setup()
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -13910,6 +13910,11 @@ obuf@^1.0.0, obuf@^1.1.2:
resolved "https://registry.yarnpkg.com/obuf/-/obuf-1.1.2.tgz#09bea3343d41859ebd446292d11c9d4db619084e"
integrity sha512-PX1wu0AmAdPqOL1mWhqmlOd8kOIZQwGZw6rh7uby9fTc5lhaOWFLX3I6R1hrF9k3zUY40e6igsLGkDXK92LJNg==

ohash@^0.1.0:
version "0.1.0"
resolved "https://registry.yarnpkg.com/ohash/-/ohash-0.1.0.tgz#cde74db91cfb7259477a93e0959221c4e20a5881"
integrity sha512-KvclyhWseX6F2UTEEp9Qzybb0LTGorTSVufAToV5tR2B6Q64rVhKhkcU/o+mBaiqGa5+PdobtfSVelp8VOCR6A==

on-finished@^2.3.0, on-finished@~2.3.0:
version "2.3.0"
resolved "https://registry.yarnpkg.com/on-finished/-/on-finished-2.3.0.tgz#20f1336481b083cd75337992a16971aa2d906947"
Expand Down

0 comments on commit 5aba888

Please sign in to comment.