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
Changes from 3 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
49 changes: 37 additions & 12 deletions packages/ckeditor5-engine/src/conversion/mapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import ModelRange from '../model/range.js';
import ViewPosition from '../view/position.js';
import ViewRange from '../view/range.js';

import { CKEditorError, EmitterMixin } from '@ckeditor/ckeditor5-utils';
import { CKEditorError, EmitterMixin, type GetCallback } from '@ckeditor/ckeditor5-utils';

import type ViewDocumentFragment from '../view/documentfragment.js';
import type ViewElement from '../view/element.js';
Expand Down Expand Up @@ -844,6 +844,37 @@ export class MapperCache extends /* #__PURE__ */ EmitterMixin() {
*/
private _nodeToCacheListIndex = new WeakMap<ViewNode, number>();

/**
* Callback fired whenever there is a direct or indirect children change in tracked view element or tracked view document fragment.
*
* This is specified as a property to make it easier to set as an event callback and to later turn off that event.
*/
private _invalidateOnChildrenChangeCallback: GetCallback<ViewNodeChangeEvent>;

/**
* Callback fired whenever a view text node directly or indirectly inside a tracked view element or tracked view document fragment
* changes its text data.
*
* This is specified as a property to make it easier to set as an event callback and to later turn off that event.
*/
private _invalidateOnTextChangeCallback: GetCallback<ViewNodeChangeEvent>;

/**
* Creates an instance of mapper cache.
*/
constructor() {
super();

this._invalidateOnChildrenChangeCallback = ( evt, viewNode, data ) => {
this._clearCacheInsideParent( viewNode as ViewElement | ViewDocumentFragment, data!.index );
};

this._invalidateOnTextChangeCallback = ( evt, viewNode ) => {
// Text node has changed. Clear all the cache starting from before this text node.
this._clearCacheStartingBefore( viewNode );
};
scofalik marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Saves cache for given view position mapping <-> model offset mapping.
*
Expand Down Expand Up @@ -1013,7 +1044,7 @@ export class MapperCache extends /* #__PURE__ */ EmitterMixin() {
/**
* Starts tracking given `viewContainer`, which must be mapped to a model element or model document fragment.
*
* Note, that this method is automatically called by {@link ~MapperCache#get `MapperCache#get()`} and there is no need to call it
* Note, that this method is automatically called by {@link ~MapperCache#get `MapperCache#getClosest()`} and there is no need to call it
scofalik marked this conversation as resolved.
Show resolved Hide resolved
* manually.
*
* This method initializes the cache for `viewContainer` and adds callbacks for
Expand All @@ -1035,14 +1066,8 @@ 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<ViewNodeChangeEvent>( viewContainer, 'change:children', ( evt, viewNode, data ) => {
this._invalidateCacheOnChildrenChange( viewNode as ViewElement | ViewDocumentFragment, data!.index );
} );

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 );
} );
viewContainer.on<ViewNodeChangeEvent>( 'change:children', this._invalidateOnChildrenChangeCallback );
viewContainer.on<ViewNodeChangeEvent>( 'change:text', this._invalidateOnTextChangeCallback );

return initialCacheItem;
}
Expand All @@ -1060,9 +1085,9 @@ export class MapperCache extends /* #__PURE__ */ EmitterMixin() {
}

/**
* Invalidates cache after a change happens inside `viewParent` on given `index`.
* Invalidates cache inside `viewParent`, starting from given `index` in that parent.
*/
private _invalidateCacheOnChildrenChange( viewParent: ViewElement | ViewDocumentFragment, index: number ) {
private _clearCacheInsideParent( viewParent: ViewElement | ViewDocumentFragment, index: number ) {
if ( index == 0 ) {
// Change at the beginning of the parent.
if ( this._cachedMapping.has( viewParent ) ) {
Expand Down
Loading