Improve Angular performance

BeratS commented 2 years ago

Describe the bug

I analyze the code how is implemented, there are a lot of function iteration calls, the whole schema its updated even in click on select option control, on focus, if you inspect in chrome, you will see the performance issues, I don't know why all the logic of updates are not set up in services. In comparison of React - in React it works ok.

Expected behavior

/

Steps to reproduce the issue

/

Screenshots

No response

In which browser are you experiencing the issue?

Google Chrome 100

Framework

Angular

RendererSet

Material

Additional context

No response

sdirix commented 2 years ago

Hi @BeratS,

we're always open to performance improvements. Please let us know when you found a concrete issue and/or want to present some improvements ranging from an improved architecture to concrete contributions.

derdeka commented 2 years ago

@sdirix

I also see a huge performance impact if the form has many fields and a large schema. The input fields take several seconds to show up the changes, which makes the app totally unuseable.

I think this line is causing this performance issue, as cloneDeep is invoked several thousand times in my setup: https://github.com/eclipsesource/jsonforms/blob/5d050129188723bee2537132132f6a9baf5a83c7/packages/angular/src/jsonforms.service.ts#L200 Is it really required to clone the state every time? When i return the original object or a shallow copy, the performance issues is almost gone. Can an observeable or an alternative way to read the locale make things better?

BeratS commented 2 years ago

`ts

/* The MIT License

Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ import { Actions, configReducer, CoreActions, coreReducer, generateDefaultUISchema, generateJsonSchema, i18nReducer, JsonFormsRendererRegistryEntry, JsonFormsState, JsonFormsSubStates, JsonSchema, I18nActions, RankedTester, setConfig, SetConfigAction, UISchemaActions, UISchemaElement, uischemaRegistryReducer, UISchemaTester, ValidationMode, updateI18n } from '@jsonforms/core'; import { BehaviorSubject, Observable } from 'rxjs'; import { JsonFormsBaseRenderer } from './base.renderer';

import { cloneDeep } from 'lodash'; import Ajv from 'ajv';

export const USE_STATE_VALUE = Symbol('Marker to use state value'); export class JsonFormsAngularService {

private _state: JsonFormsSubStates;
private state: BehaviorSubject<JsonFormsState>;
private clonedState = { jsonforms: {} as JsonFormsSubStates };

init(initialState: JsonFormsSubStates = { core: { data: undefined, schema: undefined, uischema: undefined, validationMode: 'ValidateAndShow' } }) {
    this._state = initialState;
    this._state.config = configReducer(undefined, setConfig(this._state.config));
    this._state.i18n = i18nReducer(this._state.i18n, updateI18n(this._state.i18n?.locale, this._state.i18n?.translate, this._state.i18n?.translateError));
    this.state = new BehaviorSubject({ jsonforms: this._state });
    const data = initialState.core.data;
    const schema = initialState.core.schema ?? generateJsonSchema(data);
    const uischema = initialState.core.uischema ?? generateDefaultUISchema(schema);
    this.updateCore(Actions.init(data, schema, uischema));
}

get $state(): Observable<JsonFormsState> {
    if (!this.state) {
        throw new Error('Please call init first!');
    }
    return this.state.asObservable();
}

/**
 * @deprecated use {@link JsonFormsAngularService.addRenderer}
 */
registerRenderer(renderer: JsonFormsBaseRenderer<UISchemaElement>, tester: RankedTester): void {
    this.addRenderer(renderer, tester);
}
addRenderer(renderer: JsonFormsBaseRenderer<UISchemaElement>, tester: RankedTester): void {
    this._state.renderers.push({ renderer, tester });
    this.updateSubject();
}

/**
 * @deprecated use {@link JsonFormsAngularService.setRenderer}
 */
registerRenderers(renderers: JsonFormsRendererRegistryEntry[]): void {
    this.setRenderers(renderers);
}
setRenderers(renderers: JsonFormsRendererRegistryEntry[]): void {
    this._state.renderers = renderers;
    this.updateSubject();
}

/**
 * @deprecated use {@link JsonFormsAngularService.removeRenderer}
 */
unregisterRenderer(tester: RankedTester): void {
    this.removeRenderer(tester);
}
removeRenderer(tester: RankedTester): void {
    const findIndex = this._state.renderers.findIndex(v => v.tester === tester);
    if (findIndex === -1) {
        return;
    }
    const renderers = this._state.renderers.filter(v => v.tester !== tester);
    this._state.renderers = renderers;
    this.updateSubject();
}

updateValidationMode(validationMode: ValidationMode): void {
    const coreState = coreReducer(this._state.core, Actions.setValidationMode(validationMode));
    this._state.core = coreState;
    this.updateSubject();
}

updateI18n<T extends I18nActions>(i18nAction: T): T {
    const i18nState = i18nReducer(this._state.i18n, i18nAction);
    if (i18nState !== this._state.i18n) {
        this._state.i18n = i18nState;
        this.updateSubject();
    }
    return i18nAction;
}

updateCore<T extends CoreActions>(coreAction: T): T {
    const coreState = coreReducer(this._state.core, coreAction);

    if (coreState !== this._state.core) {
        this._state.core = coreState;
        this.updateSubject();
    }
    return coreAction;
}

/**
 * @deprecated use {@link JsonFormsAngularService.setUiSchemas}
 */
updateUiSchema<T extends UISchemaActions>(uischemaAction: T): T {
    const uischemaState = uischemaRegistryReducer(this._state.uischemas, uischemaAction);
    this._state.uischemas = uischemaState;
    this.updateSubject();
    return uischemaAction;
}

setUiSchemas(uischemas: { tester: UISchemaTester; uischema: UISchemaElement; }[]): void {
    this._state.uischemas = uischemas;
    this.updateSubject();
}

updateConfig<T extends SetConfigAction>(setConfigAction: T): T {
    const configState = configReducer(this._state.config, setConfigAction);
    this._state.config = configState;
    this.updateSubject();
    return setConfigAction;
}

setUiSchema(uischema: UISchemaElement | undefined): void {
    const newUiSchema = uischema ?? generateDefaultUISchema(this._state.core.schema);
    const coreState = coreReducer(this._state.core, Actions.updateCore(this._state.core.data, this._state.core.schema, newUiSchema));
    if(coreState !== this._state.core) {
        this._state.core = coreState;
        this.updateSubject();
    }
}

setSchema(schema: JsonSchema | undefined): void {
    const coreState = coreReducer(this._state.core,
        Actions.updateCore(this._state.core.data, schema ?? generateJsonSchema(this._state.core.data), this._state.core.uischema)
    );
    if(coreState !== this._state.core) {
        this._state.core = coreState;
        this.updateSubject();
    }
}

setData(data: any): void {
    const coreState = coreReducer(this._state.core, Actions.updateCore(data, this._state.core.schema, this._state.core.uischema));
    if(coreState !== this._state.core) {
        this._state.core = coreState;
        this.updateSubject();
    }
}

setLocale(locale: string): void {
    this._state.i18n.locale = locale;
    this.updateSubject();
}

setReadonly(readonly: boolean): void {
    this._state.readonly = readonly;
    this.updateSubject();
}

getState(): JsonFormsState {
    return this.clonedState;
}

refresh(): void {
    this.updateSubject();
}

updateCoreState(
    data: any | typeof USE_STATE_VALUE,
    schema: JsonSchema | typeof USE_STATE_VALUE,
    uischema: UISchemaElement | typeof USE_STATE_VALUE,
    ajv: Ajv | typeof USE_STATE_VALUE,
    validationMode: ValidationMode | typeof USE_STATE_VALUE
): void {
    const newData = data === USE_STATE_VALUE ? this._state.core.data : data;
    const newSchema = schema === USE_STATE_VALUE ? this._state.core.schema : schema ?? generateJsonSchema(newData);
    const newUischema = uischema === USE_STATE_VALUE ? this._state.core.uischema : uischema ?? generateDefaultUISchema(newSchema);
    const newAjv = ajv === USE_STATE_VALUE ? this._state.core.ajv : ajv;
    const newValidationMode = validationMode === USE_STATE_VALUE ? this._state.core.validationMode : validationMode;
    this.updateCore(
        Actions.updateCore(newData, newSchema, newUischema, {ajv: newAjv, validationMode: newValidationMode})
    );
}

private updateSubject(): void {
    this.clonedState = cloneDeep({ jsonforms: this._state });
    this.state.next({ jsonforms: this._state });
}

}

`

BeratS commented 2 years ago

I came across to this performance improvement solution.

BeratS commented 2 years ago

`ts /* The MIT License

Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ import maxBy from 'lodash/maxBy'; import { ComponentFactoryResolver, Directive, Input, OnDestroy, OnInit, Type, ViewContainerRef } from '@angular/core'; import { createId, isControl, JsonFormsProps, JsonFormsState, JsonSchema, mapStateToJsonFormsRendererProps, OwnPropsOfRenderer, StatePropsOfJsonFormsRenderer, UISchemaElement } from '@jsonforms/core'; import { UnknownRenderer } from './unknown.component'; import { JsonFormsBaseRenderer } from './base.renderer'; import { Subscription } from 'rxjs'; import { JsonFormsControl } from './control'; import { JsonFormsAngularService } from './jsonforms.service';

import { get, isEqual } from 'lodash';

const areEqual = (prevProps: StatePropsOfJsonFormsRenderer, nextProps: StatePropsOfJsonFormsRenderer) => { return get(prevProps, 'renderers.length') === get(nextProps, 'renderers.length') && get(prevProps, 'cells.length') === get(nextProps, 'cells.length') && get(prevProps, 'uischemas.length') === get(nextProps, 'uischemas.length') && get(prevProps, 'schema') === get(nextProps, 'schema') && isEqual(get(prevProps, 'uischema'), get(nextProps, 'uischema')) && get(prevProps, 'path') === get(nextProps, 'path'); };

@Directive({ selector: 'jsonforms-outlet' }) export class JsonFormsOutlet extends JsonFormsBaseRenderer implements OnInit, OnDestroy { private subscription: Subscription; private previousProps: StatePropsOfJsonFormsRenderer; private previousRenderProps: any;

constructor( private viewContainerRef: ViewContainerRef, private componentFactoryResolver: ComponentFactoryResolver, private jsonformsService: JsonFormsAngularService ) { super(); }

@Input() set renderProps(renderProps: OwnPropsOfRenderer) {

if (this.previousRenderProps && areEqual(this.previousRenderProps, (renderProps as any))) {
  return;
}
// Store the previous state of renderProps
this.previousRenderProps = renderProps;

this.path = renderProps.path;
this.schema = renderProps.schema;
this.uischema = renderProps.uischema;
this.update(this.jsonformsService.getState());

}

ngOnInit(): void { this.subscription = this.jsonformsService.$state.subscribe({ next: (state: JsonFormsState) => this.update(state) }); }

update(state: JsonFormsState) { const props = mapStateToJsonFormsRendererProps(state, { schema: this.schema, uischema: this.uischema, path: this.path }); if (areEqual(this.previousProps, props)) { return; } else { this.previousProps = props; }

const { renderers } = props as JsonFormsProps;
const schema: JsonSchema = this.schema || props.schema;
const uischema = this.uischema || props.uischema;
const rootSchema = props.rootSchema;

const renderer = maxBy(renderers, r => r.tester(uischema, schema, rootSchema));
let bestComponent: Type<any> = UnknownRenderer;
if (renderer !== undefined && renderer.tester(uischema, schema, rootSchema) !== -1) {
  bestComponent = renderer.renderer;
}

const componentFactory = this.componentFactoryResolver.resolveComponentFactory(bestComponent);
this.viewContainerRef.clear();
const currentComponentRef = this.viewContainerRef.createComponent(componentFactory);

if (currentComponentRef.instance instanceof JsonFormsBaseRenderer) {
  const instance = currentComponentRef.instance as JsonFormsBaseRenderer<UISchemaElement>;
  instance.uischema = uischema;
  instance.schema = schema;
  instance.path = this.path;
  if (instance instanceof JsonFormsControl) {
    const controlInstance = instance as JsonFormsControl;
    if (controlInstance.id === undefined) {
      const id = isControl(props.uischema)
        ? createId(props.uischema.scope)
        : undefined;
      (instance as JsonFormsControl).id = id;
    }
  }
}

}

ngOnDestroy() { if (this.subscription) { this.subscription.unsubscribe(); } } }

`

BeratS commented 2 years ago

And another improvement that is done, now much better renders the form a bit speed up, but there also need a lot refactor, this could be done just by rxjs service in clean way not to extend abstract class, as in complex large form and validation still need time to render, There's no need browser DOM to fill in up if the component control is not showing rule: SHOW | HIDE.

derdeka commented 2 years ago

@BeratS Thanks for your feedback. Do you have a public fork of this repository to review and discuss the improvements?

BeratS commented 2 years ago

Hi Derdeka, no at the moment, I have build them and using locally as npm libraries, internal project, but I think that the code need to be updated to this repo.

sdirix commented 2 years ago

@sdirix

I also see a huge performance impact if the form has many fields and a large schema. The input fields take several seconds to show up the changes, which makes the app totally unuseable.

I think this line is causing this performance issue, as cloneDeep is invoked several thousand times in my setup:

https://github.com/eclipsesource/jsonforms/blob/5d050129188723bee2537132132f6a9baf5a83c7/packages/angular/src/jsonforms.service.ts#L200

Is it really required to clone the state every time? When i return the original object or a shallow copy, the performance issues is almost gone. Can an observeable or an alternative way to read the locale make things better?

Good find. I think this could be removed.

sdirix commented 2 years ago

@derdeka @BeratS Thanks for taking time to look into this. We are definitely interested in getting Angular performance improvements in however it's not an high priority item at the moment and therefore it could take quite a while until we can work in this direction.

The fastest way to get these changes into JSON Forms is to contribute an appropriate PR adapting the code and modifying/adding new test cases where necessary. If more than one person tested the PR beforehand this would increase confidence a lot. If the PR is of high quality we'll definitely spend effort to review/merge the changes in as soon as possible.

In an ideal world we would stay backward compatible. If that's not possible we also need an entry in the migration guide.

eclipsesource / jsonforms