Skip to content

Commit

Permalink
Merge pull request #626 from namecheap/perf/fix-cpu-perf
Browse files Browse the repository at this point in the history
perf: fix cpu usage in cache
  • Loading branch information
stas-nc authored Nov 18, 2024
2 parents c94b984 + 980fc39 commit 139f77f
Show file tree
Hide file tree
Showing 41 changed files with 628 additions and 327 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,8 @@ jobs:
fullCoverageDiff: true
autorun: false
postComment: false
oldCodeCoveragePath: ./dest-client-side-tests-artifacts/coverage/chrome/coverage-summary.json
newCodeCoveragePath: ./src-client-side-tests-artifacts/coverage/chrome/coverage-summary.json
oldCodeCoveragePath: ./dest-client-side-tests-artifacts/coverage/chrome/coverage-summary.json # remove "chrome after merge PR fix cpu
newCodeCoveragePath: ./src-client-side-tests-artifacts/coverage/coverage-summary.json
total_delta: 2
- name: Merge coverage report for registry
run: |
Expand All @@ -338,7 +338,7 @@ jobs:
postComment: false
oldCodeCoveragePath: ./dest-merged-registry-tests-artifacts/coverage/coverage-summary.json
newCodeCoveragePath: ./src-merged-registry-tests-artifacts/coverage/coverage-summary.json
total_delta: 6
total_delta: 2
- name: Compose comment
if: success() || failure()
run: |
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ node_modules
registry/lde/dbfiles
.yalc
yalc.lock
.clinic
4 changes: 2 additions & 2 deletions ilc/.mocharc.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"spec": [
"server/**/*.spec.js",
"common/**/*.spec.js"
"server/**/*.spec.{js,ts}",
"common/**/*.spec.{js,ts}"
],
"require": "ts-node/register",
"timeout": 5000,
Expand Down
13 changes: 8 additions & 5 deletions ilc/build/webpack.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
/* eslint-env node */
const fs = require('fs');
const path = require('path');
const { DefinePlugin } = require('webpack');
const WrapperPlugin = require('wrapper-webpack-plugin');
const { DefinePlugin, BannerPlugin, Compilation } = require('webpack');
const { DuplicateIlcPluginsWebpackPlugin, ResolveIlcDefaultPluginsWebpackPlugin } = require('ilc-plugins-sdk/webpack');

const { Environment } = require('../common/Environment');
const ilcPluginsPath = path.resolve(__dirname, '../../node_modules/');

const environment = new Environment(process.env);

const systemJsBundleFile = path.resolve(__dirname, '../public/system.js');
const systemJsBanner = () => fs.readFileSync(systemJsBundleFile, 'utf-8');

module.exports = {
entry: path.resolve(__dirname, '../client.js'),
output: {
Expand Down Expand Up @@ -47,10 +49,11 @@ module.exports = {
},
plugins: [
new DuplicateIlcPluginsWebpackPlugin(ilcPluginsPath),
new WrapperPlugin({
new BannerPlugin({
test: /\.js$/,
header: () => fs.readFileSync(path.resolve(__dirname, '../public/system.js')),
afterOptimizations: true,
banner: systemJsBanner,
raw: true,
stage: Compilation.PROCESS_ASSETS_STAGE_REPORT + 1,
}),
new DefinePlugin({
LEGACY_PLUGINS_DISCOVERY_ENABLED: JSON.stringify(environment.isLegacyPluginsDiscoveryEnabled()),
Expand Down
11 changes: 11 additions & 0 deletions ilc/build/webpack.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,17 @@ const config = {
},
};

config.module.rules.push({
test: /\.(js|ts)$/,
/**
* Control coveraage files
*/
exclude: /(node_modules|\.spec\.(js|ts)$|tests\/)/,
loader: '@jsdevtools/coverage-istanbul-loader',
enforce: 'post',
options: { esModules: true },
});

config.resolve.alias['nock'] = false;
config.resolve.alias['timers'] = false;

Expand Down
6 changes: 3 additions & 3 deletions ilc/client/Client.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { PluginManager } from 'ilc-plugins-sdk/browser';

import { PluginsLoader } from './PluginsLoader';
import UrlProcessor from '../common/UrlProcessor';
import { appIdToNameAndSlot, removeQueryParams, addTrailingSlashToPath } from '../common/utils';
import { appIdToNameAndSlot, addTrailingSlashToPath } from '../common/utils';

import { setNavigationErrorHandler, addNavigationHook } from './navigationEvents/setupEvents';

Expand All @@ -20,7 +20,7 @@ import {

import { triggerAppChange } from './navigationEvents';

import registryService from './registry/factory';
import { registryFactory } from './registry/factory';

import Router from './ClientRouter';
import initIlcState from './initIlcState';
Expand Down Expand Up @@ -72,7 +72,6 @@ export class Client {

constructor(config) {
this.#configRoot = config;
this.#registryService = registryService;

const pluginsLoader = new PluginsLoader();
this.#pluginManager = new PluginManager(...pluginsLoader.load());
Expand All @@ -81,6 +80,7 @@ export class Client {

this.#logger = reportingPlugin.logger;

this.#registryService = registryFactory(this.#logger);
this.#errorHandlerManager = new ErrorHandlerManager(this.#logger, this.#registryService);

const transitionTimeout = 60000;
Expand Down
1 change: 0 additions & 1 deletion ilc/client/TransitionManager/TransitionBlocker.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ describe('TransitionBlocker', () => {

it('Should set onDestroy callback', () => {
transitionBlocker.destroy();
console.log(cancelTokenSpy.callCount);
expect(cancelTokenSpy.callCount).to.be.equal(1);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ describe('TransitionManager Force Block removal', () => {

afterEach(() => {
slots.removeSlots();
console.log(document.body.innerHTML);
clock.restore();
removePageTransactionListeners();
logger.warn.resetHistory();
Expand Down
1 change: 0 additions & 1 deletion ilc/client/configuration/IlcConfigRoot.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { getIlcConfigRoot } from './getIlcConfigRoot';
describe('IlcConfigRoot', () => {
it('IlcConfigRoot should init', () => {
const configRoot = getIlcConfigRoot();
console.log(configRoot.getConfigForApps());
expect(configRoot).to.be.an('object');
});
it('IlcConfigRoot should return singleton', () => {
Expand Down
65 changes: 65 additions & 0 deletions ilc/client/registry/BrowserCacheStorage.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { expect } from 'chai';
import sinon from 'sinon';
import { BrowserCacheStorage } from './BrowserCacheStorage';

describe('BrowserCacheStorage', () => {
let cacheStorage: BrowserCacheStorage;
let mockLocalStorage: Storage;

const sandbox = sinon.createSandbox();

beforeEach(() => {
// Mock the localStorage API
mockLocalStorage = {
getItem: sandbox.stub(),
setItem: sandbox.stub(),
removeItem: sandbox.stub(),
clear: sandbox.stub(),
} as unknown as Storage;

cacheStorage = new BrowserCacheStorage(mockLocalStorage);
});

afterEach(() => {
// Restore the default sandbox here
sandbox.restore();
});

describe('getItem', () => {
it('should return parsed data from localStorage if key exists', () => {
const key = 'testKey';
const value = { data: 'testData' };

// Mock localStorage.getItem to return stringified data
(mockLocalStorage.getItem as sinon.SinonStub).withArgs(key).returns(JSON.stringify(value));

const result = cacheStorage.getItem<typeof value>(key);

sandbox.assert.calledWith(mockLocalStorage.getItem as sinon.SinonStub, key);
expect(result).to.deep.equal(value);
});

it('should return null if key does not exist in localStorage', () => {
const key = 'missingKey';

// Mock localStorage.getItem to return null
(mockLocalStorage.getItem as sinon.SinonStub).withArgs(key).returns(null);

const result = cacheStorage.getItem(key);

sandbox.assert.calledWith(mockLocalStorage.getItem as sinon.SinonStub, key);
expect(result).to.be.null;
});
});

describe('setItem', () => {
it('should store stringified data in localStorage', () => {
const key = 'testKey';
const value = { data: 'testData', cachedAt: Date.now() };

cacheStorage.setItem(key, value);

sandbox.assert.calledWith(mockLocalStorage.setItem as sinon.SinonStub, key, JSON.stringify(value));
});
});
});
14 changes: 14 additions & 0 deletions ilc/client/registry/BrowserCacheStorage.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { CacheResult, CacheStorage } from '../../common/types/CacheWrapper';

export class BrowserCacheStorage implements CacheStorage {
constructor(private readonly storage: Storage) {}

getItem<T>(key: string): CacheResult<T> {
const cache = this.storage.getItem(key);
return cache ? JSON.parse(cache) : null;
}

setItem(key: string, cache: CacheResult<unknown>): void {
this.storage.setItem(key, JSON.stringify(cache));
}
}
5 changes: 0 additions & 5 deletions ilc/client/registry/factory.js

This file was deleted.

8 changes: 8 additions & 0 deletions ilc/client/registry/factory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import type { Logger } from 'ilc-plugins-sdk';
import type { IlcLogger } from 'ilc-plugins-sdk/browser';
import { DefaultCacheWrapper } from '../../common/DefaultCacheWrapper';
import { BrowserCacheStorage } from './BrowserCacheStorage';
import Registry from './Registry';

export const registryFactory = (logger: IlcLogger) =>
new Registry(new DefaultCacheWrapper(new BrowserCacheStorage(window.localStorage), logger as Logger));
Loading

0 comments on commit 139f77f

Please sign in to comment.