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

Dynamic caching in Mapper for better performance #17586

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
10 changes: 5 additions & 5 deletions packages/ckeditor5-engine/src/conversion/mapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import type ViewElement from '../view/element.js';
import type ViewText from '../view/text.js';
import type ModelElement from '../model/element.js';
import type ModelDocumentFragment from '../model/documentfragment.js';
import type ViewNode from '../view/node.js';
import type { default as ViewNode, ViewNodeChangeChildrenEvent, ViewNodeChangeEvent } from '../view/node.js';

/**
* Maps elements, positions and markers between the {@link module:engine/view/document~Document view} and
Expand Down Expand Up @@ -990,13 +990,13 @@ export class MapperCache extends /* #__PURE__ */ EmitterMixin() {
//
// Possible performance improvement. This event bubbles, so if there are multiple tracked (mapped) elements that are ancestors
// then this will be unnecessarily fired for each ancestor. This could be rewritten to listen only to roots and document fragments.
this.listenTo( viewContainer, 'change:children', ( evt, viewNode, data ) => {
this._invalidateCacheOnChildrenChange( viewNode as ViewElement | ViewDocumentFragment, ( data as any ).index as number );
this.listenTo<ViewNodeChangeChildrenEvent>( viewContainer, 'change:children', ( evt, viewNode, data ) => {
this._invalidateCacheOnChildrenChange( viewNode, data.index );
} );

this.listenTo( viewContainer, 'change:text', ( evt, viewNode ) => {
this.listenTo<ViewNodeChangeEvent>( viewContainer, 'change:text', ( evt, viewNode ) => {
// Text node has changed. Clear all the cache starting from before this text node.
this._clearCacheStartingBefore( viewNode as ViewText );
this._clearCacheStartingBefore( viewNode );
} );

return initialCacheItem;
Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-engine/src/view/documentfragment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ export default class DocumentFragment extends /* #__PURE__ */ EmitterMixin( Type
* @param data Additional data.
* @fires change
*/
public _fireChange( type: ChangeType, node: Node | DocumentFragment, data?: unknown ): void {
public _fireChange( type: ChangeType, node: Node | DocumentFragment, data?: { index: number } ): void {
this.fire( `change:${ type }`, node, data );
}

Expand Down
39 changes: 28 additions & 11 deletions packages/ckeditor5-engine/src/view/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,12 @@ export default abstract class Node extends /* #__PURE__ */ EmitterMixin( TypeChe
* @param data Additional data.
* @fires change
*/
public _fireChange( type: ChangeType, node: Node, data?: unknown ): void {
this.fire<ViewNodeChangeEvent>( `change:${ type }`, node, data );
public _fireChange( type: ChangeType, node: Node, data?: { index: number } ): void {
if ( type == 'children' ) {
this.fire( 'change:children', node, data! );
} else {
this.fire( `change:${ type }`, node );
}

if ( this.parent ) {
this.parent._fireChange( type, node, data );
Expand Down Expand Up @@ -305,20 +309,33 @@ Node.prototype.is = function( type: string ): boolean {
};

/**
* Fired when list of {@link module:engine/view/element~Element elements} children, attributes or text changes.
*
* Change event is bubbled – it is fired on all ancestors.
* Fired when node attributes or text changes.
*
* All change events as the first parameter receive the node that has changed (the node for which children, attributes or text changed).
* This event is bubbled – it is fired on all ancestors of the changed node.
*
* If `change:children` event is fired, there is an additional second parameter, which is an object with additional data related to change.
* The first parameter and only parameter is the node that has changed (the node for which attributes or text changed).
*
* @eventName ~Node#change
* @eventName ~Node#change:children
* @eventName ~Node#change:attributes
* @eventName ~Node#change:text
* @eventName ~Node#change:attributes
*/
export type ViewNodeChangeEvent = {
name: 'change' | `change:${ ChangeType }`;
args: [ changedNode: Node, data?: unknown ];
name: 'change:text' | 'change:attributes';
args: [ changedNode: Node ];
};

/**
* Fired when the list of {@link module:engine/view/element~Element element's} children changes (i.e. a child is added to or removed from
* an element). If multiple children are added or removed, there is only one event.
*
* This event is bubbled – it is fired on all ancestors of the changed element.
*
* The first parameter is the element that has changed (the element for which children list changed).
* The second parameter is an object with `index` property, which informs on which index the change happened.
*
* @eventName ~Node#change:children
*/
export type ViewNodeChangeChildrenEvent = {
name: 'change:children';
args: [ changedNode: Element | DocumentFragment, data: { index: number } ];
scofalik marked this conversation as resolved.
Show resolved Hide resolved
};
Loading