IMPROVE property access performance (#5063)

* IMPROVE property access performance

* FIX #4949 RxDocument.get() on additonalProperty

* IMPROVE performance

* IMPROVE performance

* FIX bun

docs-src/releases/15.0.0.md

@@ -79,3 +79,7 @@ I will add more p2p replication plugins in the future, which are not based on [W
 
 - Added RxStorage.info() which returns the total amount of stored documents.
 - Change response type of RxStorageInstance.bulkWrite() from indexeddb objects to arrays for better performance.
+
+## Improve RxDocument property access performance
+
+We now use the Proxy API instead of defining getters on each nested property. Also fixed [#4949](https://github.com/pubkey/rxdb/pull/4949)

orga/performance-trackings.md

@@ -1522,3 +1522,18 @@ AFTER2: (lazy writes)
     "find-by-query-parallel-4": 2.74,
     "count": 0.72
 }
+
+
+## Improve property access time (04 October 2023)
+
+> npm run test:performance:memory:node
+
+BEFORE:
+"property-access": 6.97
+"property-access": 7.21
+
+AFTER:
+"property-access": 4.71
+"property-access": 5.14
+"property-access": 5.07
+"property-access": 4.56

src/custom-index.ts

@@ -56,7 +56,6 @@ export function getIndexMeta<RxDocType>(
             fieldName
         );
         if (!schemaPart) {
-            console.dir(schema);
             throw new Error('not in schema: ' + fieldName);
         }
         const type = schemaPart.type;

src/plugins/utils/utils-object-dot-prop.ts

@@ -182,11 +182,26 @@ function assertNotStringIndex(object: any, key: string | number) {
     }
 }
 
+/**
+ * TODO we need some performance tests and improvements here.
+ */
 export function getProperty(object: any, path: string | string[], value?: any) {
     if (Array.isArray(path)) {
         path = path.join('.');
     }
 
+    /**
+     * Performance shortcut.
+     * In most cases we just have a simple property name
+     * so we can directly return it.
+     */
+    if (
+        !path.includes('.') &&
+        !path.includes('[')
+    ) {
+        return object[path];
+    }
+
     if (!isObject(object as any) || typeof path !== 'string') {
         return value === undefined ? object : value;
     }

src/plugins/utils/utils-string.ts

@@ -46,6 +46,13 @@ export function trimDots(str: string): string {
     return str;
 }
 
+/**
+ * @link https://stackoverflow.com/a/44950500/3443137
+ */
+export function lastCharOfString(str: string): string {
+    return str.charAt(str.length - 1);
+}
+
 /**
  * returns true if the given name is likely a folder path
  */

src/rx-database-internal-store.ts

@@ -189,6 +189,7 @@ export async function ensureStorageTokenDocumentExists<Collections extends Colle
         return writeResult.success[0];
     }
 
+
     /**
      * If we get a 409 error,
      * it means another instance already inserted the storage token.
@@ -203,7 +204,7 @@ export async function ensureStorageTokenDocumentExists<Collections extends Colle
         if (
             passwordHash &&
             passwordHash !== conflictError.documentInDb.data.passwordHash
-        ) {
+            ) {
             throw newRxError('DB1', {
                 passwordHash,
                 existingPasswordHash: conflictError.documentInDb.data.passwordHash

src/rx-document.ts

@@ -15,7 +15,8 @@ import {
     flatClone,
     PROMISE_RESOLVE_NULL,
     RXJS_SHARE_REPLAY_DEFAULTS,
-    getProperty
+    getProperty,
+    getFromMapOrCreate
 } from './plugins/utils';
 import {
     newRxError
@@ -125,6 +126,7 @@ export const basePrototype = {
                 this.collection.schema.jsonSchema,
                 path
             );
+
             if (!schemaObj) {
                 throw newRxError('DOC4', {
                     path
@@ -183,40 +185,52 @@ export const basePrototype = {
     },
     /**
      * get data by objectPath
+     * @hotPath Performance here is really important,
+     * run some tests before changing anything.
      */
     get(this: RxDocument, objPath: string): any | null {
-        if (!this._data) {
-            return undefined;
-        }
-
-        const fromCache = this._propertyCache.get(objPath);
-        if (fromCache) {
-            return fromCache;
-        }
-
-        let valueObj = getProperty(this._data, objPath);
-
-        // direct return if array or non-object
-        if (
-            typeof valueObj !== 'object' ||
-            Array.isArray(valueObj)
-        ) {
-            return overwritable.deepFreezeWhenDevMode(valueObj);
-        }
-
-        /**
-         * TODO find a way to deep-freeze together with defineGetterSetter
-         * so we do not have to do a deep clone here.
-         */
-        valueObj = clone(valueObj);
-        defineGetterSetter(
-            this.collection.schema,
-            valueObj,
+        return getFromMapOrCreate(
+            this._propertyCache,
             objPath,
-            this as any
+            () => {
+                const valueObj = getProperty(this._data, objPath);
+
+                // direct return if array or non-object
+                if (
+                    typeof valueObj !== 'object' ||
+                    Array.isArray(valueObj)
+                ) {
+                    return overwritable.deepFreezeWhenDevMode(valueObj);
+                }
+                const _this = this;
+                const proxy = new Proxy(
+                    /**
+                     * In dev-mode, the _data is deep-frozen
+                     * so we have to flat clone here so that
+                     * the proxy can work.
+                     */
+                    flatClone(valueObj),
+                    {
+                        get(target, property: any) {
+                            if (typeof property !== 'string') {
+                                return target[property];
+                            }
+                            const lastChar = property.charAt(property.length - 1);
+                            if (lastChar === '$') {
+                                const key = property.slice(0, -1);
+                                return _this.get$(trimDots(objPath + '.' + key));
+                            } else if (lastChar === '_') {
+                                const key = property.slice(0, -1);
+                                return _this.populate(trimDots(objPath + '.' + key));
+                            } else {
+                                return _this.get(trimDots(objPath + '.' + property));
+                            }
+                        }
+                    });
+                return proxy;
+            }
         );
-        this._propertyCache.set(objPath, valueObj);
-        return valueObj;
+
     },
 
     toJSON(this: RxDocument, withMetaFields = false) {
@@ -425,71 +439,6 @@ export function createRxDocumentConstructor(proto = basePrototype) {
     return constructor;
 }
 
-export function defineGetterSetter(
-    schema: any,
-    valueObj: any,
-    objPath = '',
-    thisObj = false
-) {
-    if (valueObj === null) {
-        return;
-    }
-
-
-    let pathProperties = getSchemaByObjectPath(
-        schema.jsonSchema,
-        objPath
-    );
-
-    if (typeof pathProperties === 'undefined') {
-        return;
-    }
-    if (pathProperties.properties) {
-        pathProperties = pathProperties.properties;
-    }
-
-    Object.keys(pathProperties)
-        .forEach(key => {
-            const fullPath = trimDots(objPath + '.' + key);
-
-            // getter - value
-            valueObj.__defineGetter__(
-                key,
-                function (this: RxDocument) {
-                    const _this: RxDocument = thisObj ? thisObj : (this as any);
-                    if (!_this.get || typeof _this.get !== 'function') {
-                        /**
-                         * When an object gets added to the state of a vuejs-component,
-                         * it happens that this getter is called with another scope.
-                         * To prevent errors, we have to return undefined in this case
-                         */
-                        return undefined;
-                    }
-                    const ret = _this.get(fullPath);
-                    return ret;
-                }
-            );
-            // getter - observable$
-            Object.defineProperty(valueObj, key + '$', {
-                get: function () {
-                    const _this = thisObj ? thisObj : this;
-                    return _this.get$(fullPath);
-                },
-                enumerable: false,
-                configurable: false
-            });
-            // getter - populate_
-            Object.defineProperty(valueObj, key + '_', {
-                get: function () {
-                    const _this = thisObj ? thisObj : this;
-                    return _this.populate(fullPath);
-                },
-                enumerable: false,
-                configurable: false
-            });
-        });
-}
-
 export function createWithConstructor<RxDocType>(
     constructor: any,
     collection: RxCollection<RxDocType>,
@@ -528,3 +477,4 @@ export function beforeDocumentUpdateWrite<RxDocType>(
     }
     return collection._runHooks('pre', 'save', newData, oldData);
 }
+

src/rx-schema.ts

@@ -9,15 +9,13 @@ import {
 import {
     runPluginHooks
 } from './hooks';
-import {
-    defineGetterSetter
-} from './rx-document';
 
 import type {
     DeepMutable,
     DeepReadonly,
     HashFunction,
     MaybeReadonly,
+    RxDocument,
     RxDocumentData,
     RxJsonSchema,
     StringKeys
@@ -27,6 +25,7 @@ import {
     getComposedPrimaryKeyOfDocumentData,
     getFinalFields,
     getPrimaryFieldOfPrimaryKey,
+    getSchemaByObjectPath,
     normalizeRxJsonSchema
 } from './rx-schema-helper';
 import { overwritable } from './overwritable';
@@ -103,8 +102,55 @@ export class RxSchema<RxDocType = any> {
      * see RxCollection.getDocumentPrototype()
      */
     public getDocumentPrototype(): any {
-        const proto = {};
-        defineGetterSetter(this, proto, '');
+        const proto: any = {};
+
+        /**
+         * On the top level, we know all keys
+         * and therefore do not have to create a new Proxy object
+         * for each document. Instead we define the getter in the prototype once.
+         */
+        const pathProperties = getSchemaByObjectPath(
+            this.jsonSchema,
+            ''
+        );
+        Object.keys(pathProperties)
+            .forEach(key => {
+                const fullPath = key;
+
+                // getter - value
+                proto.__defineGetter__(
+                    key,
+                    function (this: RxDocument) {
+                        if (!this.get || typeof this.get !== 'function') {
+                            /**
+                             * When an object gets added to the state of a vuejs-component,
+                             * it happens that this getter is called with another scope.
+                             * To prevent errors, we have to return undefined in this case
+                             */
+                            return undefined;
+                        }
+                        const ret = this.get(fullPath);
+                        return ret;
+                    }
+                );
+                // getter - observable$
+                Object.defineProperty(proto, key + '$', {
+                    get: function () {
+                        return this.get$(fullPath);
+                    },
+                    enumerable: false,
+                    configurable: false
+                });
+                // getter - populate_
+                Object.defineProperty(proto, key + '_', {
+                    get: function () {
+                        return this.populate(fullPath);
+                    },
+                    enumerable: false,
+                    configurable: false
+                });
+            });
+
         overwriteGetterForCaching(
             this,
             'getDocumentPrototype',

test/helper/schema-objects.ts

@@ -352,6 +352,9 @@ export interface AverageSchemaDocumentType {
     deep: {
         deep1: string;
         deep2: string;
+        deeper: {
+            deepNr: number;
+        };
     };
     list: {
         deep1: string;
@@ -369,7 +372,10 @@ export function averageSchema(
             var2: randomNumber(100, 50000),
             deep: {
                 deep1: randomString(5),
-                deep2: randomString(8)
+                deep2: randomString(8),
+                deeper: {
+                    deepNr: randomNumber(0, 10)
+                }
             },
             list: new Array(5).fill(0).map(() => ({
                 deep1: randomString(5),