Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: fix cpu usage in cache #626

Merged
merged 4 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading