From d6c4dab444a14f4bcd244de58e37bca870d2cb3b Mon Sep 17 00:00:00 2001 From: flora Date: Fri, 15 Nov 2024 01:58:07 +0800 Subject: [PATCH 1/2] Performance/message memory estimation performance tests (#258) **User-Facing Changes** N/A **Description** Analyzed and investigated the estimateObjectSize() to see if would be possible to get performance gains from it, having in mind the article medium.com/@bpmxmqd/v8-engine-jsobject-structure-analysis-and-memory-optimization-ideas-be30cfcdcd16 I changed the formula that estimates memory allocation for objects that are considered big, more than 1020 properties in this case, but couldn't really achieve any performance gain from, it even got slower sometimes I've still refactored this function and created a unit test for a specific case that was tested and wasn't covered by the unit tests **Checklist** - [X] The web version was tested and it is running ok - [X] The desktop version was tested and it is running ok - [X] This change is covered by unit tests - [X] Files constants.ts, types.ts and *.style.ts have been checked and relevant code snippets have been relocated --------- Co-authored-by: Alexandre Neuwald --- packages/suite-base/src/players/constants.ts | 32 ++++ .../players/messageMemoryEstimation.test.ts | 25 ++- .../src/players/messageMemoryEstimation.ts | 156 +++++++++--------- .../testing/builders/MessageEventBuilder.ts | 28 ++++ 4 files changed, 163 insertions(+), 78 deletions(-) create mode 100644 packages/suite-base/src/players/constants.ts create mode 100644 packages/suite-base/src/testing/builders/MessageEventBuilder.ts diff --git a/packages/suite-base/src/players/constants.ts b/packages/suite-base/src/players/constants.ts new file mode 100644 index 0000000..dab5b26 --- /dev/null +++ b/packages/suite-base/src/players/constants.ts @@ -0,0 +1,32 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/ + +/** + * Values of the contants below are a (more or less) informed guesses and not guaranteed to be accurate. + */ +export const COMPRESSED_POINTER_SIZE = 4; // Pointers use 4 bytes (also on 64-bit systems) due to pointer compression +export const OBJECT_BASE_SIZE = 3 * COMPRESSED_POINTER_SIZE; // 3 compressed pointers +// Arrays have an additional length property (1 pointer) and a backing store header (2 pointers) +// See https://stackoverflow.com/a/70550693. +export const ARRAY_BASE_SIZE = OBJECT_BASE_SIZE + 3 * COMPRESSED_POINTER_SIZE; +export const TYPED_ARRAY_BASE_SIZE = 25 * COMPRESSED_POINTER_SIZE; // byteLength, byteOffset, ..., see https://stackoverflow.com/a/45808835 +export const SMALL_INTEGER_SIZE = COMPRESSED_POINTER_SIZE; // Small integers (up to 31 bits), pointer tagging +export const HEAP_NUMBER_SIZE = 8 + 2 * COMPRESSED_POINTER_SIZE; // 4-byte map pointer + 8-byte payload + property pointer +export const FIELD_SIZE_BY_PRIMITIVE: Record = { + bool: SMALL_INTEGER_SIZE, + int8: SMALL_INTEGER_SIZE, + uint8: SMALL_INTEGER_SIZE, + int16: SMALL_INTEGER_SIZE, + uint16: SMALL_INTEGER_SIZE, + int32: SMALL_INTEGER_SIZE, + uint32: SMALL_INTEGER_SIZE, + float32: HEAP_NUMBER_SIZE, + float64: HEAP_NUMBER_SIZE, + int64: HEAP_NUMBER_SIZE, + uint64: HEAP_NUMBER_SIZE, + time: OBJECT_BASE_SIZE + 2 * HEAP_NUMBER_SIZE + COMPRESSED_POINTER_SIZE, + duration: OBJECT_BASE_SIZE + 2 * HEAP_NUMBER_SIZE + COMPRESSED_POINTER_SIZE, + string: 20, // we don't know the length upfront, assume a fixed length +}; +export const MAX_NUM_FAST_PROPERTIES = 1020; diff --git a/packages/suite-base/src/players/messageMemoryEstimation.test.ts b/packages/suite-base/src/players/messageMemoryEstimation.test.ts index 6b51ed3..b4e2466 100644 --- a/packages/suite-base/src/players/messageMemoryEstimation.test.ts +++ b/packages/suite-base/src/players/messageMemoryEstimation.test.ts @@ -2,10 +2,10 @@ // License, v2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at http://mozilla.org/MPL/2.0/ +import { OBJECT_BASE_SIZE } from "./constants"; import { estimateMessageObjectSize, estimateMessageFieldSizes, - OBJECT_BASE_SIZE, estimateObjectSize, } from "./messageMemoryEstimation"; @@ -145,6 +145,29 @@ describe("memoryEstimationByObject", () => { expect(sizeInBytes).toBeGreaterThan(0); }); + it("correctly estimates the size for a large object with more than 1020 fields", () => { + const largeObject: Record = {}; + const numProps = 1021; + const propertiesDictSize = 24632; + let valueSize = 0; + + for (let i = 0; i < numProps; i++) { + if (i % 3 === 0) { + largeObject[`field${i}`] = i; + valueSize += 4; + } else if (i % 3 === 1) { + largeObject[`field${i}`] = true; + valueSize += 4; + } else { + largeObject[`field${i}`] = 1.23; + valueSize += 16; + } + } + const expectedSize = 12 + valueSize + propertiesDictSize - numProps * 4; + const sizeInBytes = estimateObjectSize(largeObject); + expect(sizeInBytes).toEqual(expectedSize); + }); + it("correctly estimates the size for a simple object", () => { const sizeInBytes = estimateObjectSize({ field1: 1, // 4 bytes, SMI (fits in pointer) diff --git a/packages/suite-base/src/players/messageMemoryEstimation.ts b/packages/suite-base/src/players/messageMemoryEstimation.ts index 233c19d..87505ec 100644 --- a/packages/suite-base/src/players/messageMemoryEstimation.ts +++ b/packages/suite-base/src/players/messageMemoryEstimation.ts @@ -5,35 +5,18 @@ import Log from "@lichtblick/log"; import { MessageDefinitionMap } from "@lichtblick/mcap-support/src/types"; +import { + COMPRESSED_POINTER_SIZE, + OBJECT_BASE_SIZE, + ARRAY_BASE_SIZE, + TYPED_ARRAY_BASE_SIZE, + SMALL_INTEGER_SIZE, + HEAP_NUMBER_SIZE, + FIELD_SIZE_BY_PRIMITIVE, + MAX_NUM_FAST_PROPERTIES, +} from "./constants"; + const log = Log.getLogger(__filename); -/** - * Values of the contants below are a (more or less) informed guesses and not guaranteed to be accurate. - */ -const COMPRESSED_POINTER_SIZE = 4; // Pointers use 4 bytes (also on 64-bit systems) due to pointer compression -export const OBJECT_BASE_SIZE = 3 * COMPRESSED_POINTER_SIZE; // 3 compressed pointers -// Arrays have an additional length property (1 pointer) and a backing store header (2 pointers) -// See https://stackoverflow.com/a/70550693. -const ARRAY_BASE_SIZE = OBJECT_BASE_SIZE + 3 * COMPRESSED_POINTER_SIZE; -const TYPED_ARRAY_BASE_SIZE = 25 * COMPRESSED_POINTER_SIZE; // byteLength, byteOffset, ..., see https://stackoverflow.com/a/45808835 -const SMALL_INTEGER_SIZE = COMPRESSED_POINTER_SIZE; // Small integers (up to 31 bits), pointer tagging -const HEAP_NUMBER_SIZE = 8 + 2 * COMPRESSED_POINTER_SIZE; // 4-byte map pointer + 8-byte payload + property pointer -const FIELD_SIZE_BY_PRIMITIVE: Record = { - bool: SMALL_INTEGER_SIZE, - int8: SMALL_INTEGER_SIZE, - uint8: SMALL_INTEGER_SIZE, - int16: SMALL_INTEGER_SIZE, - uint16: SMALL_INTEGER_SIZE, - int32: SMALL_INTEGER_SIZE, - uint32: SMALL_INTEGER_SIZE, - float32: HEAP_NUMBER_SIZE, - float64: HEAP_NUMBER_SIZE, - int64: HEAP_NUMBER_SIZE, - uint64: HEAP_NUMBER_SIZE, - time: OBJECT_BASE_SIZE + 2 * HEAP_NUMBER_SIZE + COMPRESSED_POINTER_SIZE, - duration: OBJECT_BASE_SIZE + 2 * HEAP_NUMBER_SIZE + COMPRESSED_POINTER_SIZE, - string: 20, // we don't know the length upfront, assume a fixed length -}; -const MAX_NUM_FAST_PROPERTIES = 1020; /** * Estimates the memory size of a deserialized message object based on the schema definition. @@ -203,69 +186,88 @@ export function estimateObjectSize(obj: unknown): number { if (obj == undefined) { return SMALL_INTEGER_SIZE; } + + const estimateArraySize = (array: unknown[]): number => + COMPRESSED_POINTER_SIZE + + ARRAY_BASE_SIZE + + array.reduce( + (accumulator: number, value: unknown) => accumulator + estimateObjectSize(value), + 0, + ); + + const estimateMapSize = (map: Map): number => + COMPRESSED_POINTER_SIZE + + OBJECT_BASE_SIZE + + Array.from(map.entries()).reduce( + (accumulator: number, [key, value]: [unknown, unknown]) => + accumulator + estimateObjectSize(key) + estimateObjectSize(value), + 0, + ); + + const estimateSetSize = (set: Set): number => + COMPRESSED_POINTER_SIZE + + OBJECT_BASE_SIZE + + Array.from(set.values()).reduce( + (accumulator: number, value: unknown) => accumulator + estimateObjectSize(value), + 0, + ); + + const estimateObjectPropertiesSize = (object: Record): number => { + const valuesSize = Object.values(object).reduce( + (accumulator: number, value: unknown) => accumulator + estimateObjectSize(value), + 0, + ); + const numProps = Object.keys(obj).length; + + if (numProps > MAX_NUM_FAST_PROPERTIES) { + // If there are too many properties, V8 stores Objects in dictionary mode (slow properties) + // with each object having a self-contained dictionary. This dictionary contains the key, value + // and details of properties. Below we estimate the size of this additional dictionary. Formula + // adapted from medium.com/@bpmxmqd/v8-engine-jsobject-structure-analysis-and-memory-optimization-ideas-be30cfcdcd16 + const propertiesDictSize = + 16 + 5 * 8 + 2 ** Math.ceil(Math.log2((numProps + 2) * 1.5)) * 3 * 4; + return ( + OBJECT_BASE_SIZE + valuesSize + propertiesDictSize - numProps * COMPRESSED_POINTER_SIZE + ); + } + + return OBJECT_BASE_SIZE + valuesSize; + }; + switch (typeof obj) { case "undefined": - case "boolean": { + case "boolean": return SMALL_INTEGER_SIZE; - } - case "number": { + + case "number": return Number.isInteger(obj) ? SMALL_INTEGER_SIZE : HEAP_NUMBER_SIZE; - } - case "bigint": { + + case "bigint": return HEAP_NUMBER_SIZE; - } - case "string": { - // The string length is rounded up to the next multiple of 4. + + case "string": return COMPRESSED_POINTER_SIZE + OBJECT_BASE_SIZE + Math.ceil(obj.length / 4) * 4; - } - case "object": { + + case "object": if (Array.isArray(obj)) { - return ( - COMPRESSED_POINTER_SIZE + - ARRAY_BASE_SIZE + - Object.values(obj).reduce((acc, val) => acc + estimateObjectSize(val), 0) - ); - } else if (ArrayBuffer.isView(obj)) { + return estimateArraySize(obj); + } + if (ArrayBuffer.isView(obj)) { return TYPED_ARRAY_BASE_SIZE + obj.byteLength; - } else if (obj instanceof Set) { - return ( - COMPRESSED_POINTER_SIZE + - OBJECT_BASE_SIZE + - Array.from(obj.values()).reduce((acc, val) => acc + estimateObjectSize(val), 0) - ); - } else if (obj instanceof Map) { - return ( - COMPRESSED_POINTER_SIZE + - OBJECT_BASE_SIZE + - Array.from(obj.entries()).reduce( - (acc, [key, val]) => acc + estimateObjectSize(key) + estimateObjectSize(val), - 0, - ) - ); } - - let propertiesSize = 0; - const numProps = Object.keys(obj).length; - if (numProps > MAX_NUM_FAST_PROPERTIES) { - // If there are too many properties, V8 stores Objects in dictionary mode (slow properties) - // with each object having a self-contained dictionary. This dictionary contains the key, value - // and details of properties. Below we estimate the size of this additional dictionary. Formula - // adapted from - // medium.com/@bpmxmqd/v8-engine-jsobject-structure-analysis-and-memory-optimization-ideas-be30cfcdcd16 - const propertiesDictSize = - 16 + 5 * 8 + 2 ** Math.ceil(Math.log2((numProps + 2) * 1.5)) * 3 * 4; - // In return, properties are no longer stored in the properties array, so we subtract that. - propertiesSize = propertiesDictSize - numProps * COMPRESSED_POINTER_SIZE; + if (obj instanceof Set) { + return estimateSetSize(obj); + } + if (obj instanceof Map) { + return estimateMapSize(obj); } + return estimateObjectPropertiesSize(obj as Record); - const valuesSize = Object.values(obj).reduce((acc, val) => acc + estimateObjectSize(val), 0); - return OBJECT_BASE_SIZE + propertiesSize + valuesSize; - } case "symbol": - case "function": { + case "function": throw new Error(`Can't estimate size of type '${typeof obj}'`); - } } + log.error(`Can't estimate size of type '${typeof obj}'`); return SMALL_INTEGER_SIZE; } diff --git a/packages/suite-base/src/testing/builders/MessageEventBuilder.ts b/packages/suite-base/src/testing/builders/MessageEventBuilder.ts new file mode 100644 index 0000000..819330e --- /dev/null +++ b/packages/suite-base/src/testing/builders/MessageEventBuilder.ts @@ -0,0 +1,28 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/ + +import { MessageEvent } from "@lichtblick/suite"; +import BasicBuilder from "@lichtblick/suite-base/testing/builders/BasicBuilder"; +import RosTimeBuilder from "@lichtblick/suite-base/testing/builders/RosTimeBuilder"; +import { defaults } from "@lichtblick/suite-base/testing/builders/utilities"; + +class MessageEventBuilder { + public static messageEvent(props: Partial> = {}): MessageEvent { + return defaults>(props, { + message: BasicBuilder.stringMap() as T, + publishTime: RosTimeBuilder.time(), + receiveTime: RosTimeBuilder.time(), + schemaName: BasicBuilder.string(), + sizeInBytes: BasicBuilder.number(), + topic: BasicBuilder.string(), + topicConfig: BasicBuilder.stringMap(), + }); + } + + public static messageEvents(count = 3): MessageEvent[] { + return BasicBuilder.multiple(MessageEventBuilder.messageEvent, count); + } +} + +export default MessageEventBuilder; From 784953774847bded6e5580a034660adc08bca2aa Mon Sep 17 00:00:00 2001 From: flora Date: Sat, 16 Nov 2024 01:49:10 +0800 Subject: [PATCH 2/2] Fix toggling of panels by listen only to revert event (#264) **User-Facing Changes** Users use to experience some toggling in some layouts, which made impossible to use Lichtblick. **Description** The toggling was solved by filtering the "change event" in the CurrentLayoutProvider, and update in that specific moment only if it is a revert action (revert the unsaved modifications on current layout) instead of all changes. For some reason this change event is possibly duplicated and is not necessary in this part. The tests passed and it was tested manually too. It seems that all functionality regards to the layouts is still working. **Checklist** - [x] The web version was tested and it is running ok - [x] The desktop version was tested and it is running ok - [x] Files constants.ts, types.ts and *.style.ts have been checked and relevant code snippets have been relocated --- .../src/providers/CurrentLayoutProvider/index.tsx | 11 ++++++++--- packages/suite-base/src/services/ILayoutManager.ts | 2 +- .../src/services/LayoutManager/LayoutManager.ts | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/suite-base/src/providers/CurrentLayoutProvider/index.tsx b/packages/suite-base/src/providers/CurrentLayoutProvider/index.tsx index 4a0df42..13280fc 100644 --- a/packages/suite-base/src/providers/CurrentLayoutProvider/index.tsx +++ b/packages/suite-base/src/providers/CurrentLayoutProvider/index.tsx @@ -212,11 +212,16 @@ export default function CurrentLayoutProvider({ [setLayoutState], ); - // Changes to the layout storage from external user actions (such as resetting a layout to a - // previous saved state) need to trigger setLayoutState. + /** + * Changes to the layout storage from external user actions need to trigger setLayoutState. + * Before it was beeing trigged on every change. Now it is triggered only when the layout + * is reverted, otherize it has some toggling issues when resizing panels. + */ useEffect(() => { - const listener: LayoutManagerEventTypes["change"] = ({ updatedLayout }) => { + const listener: LayoutManagerEventTypes["change"] = (event) => { + const { updatedLayout } = event; if ( + event.type === "revert" && updatedLayout && layoutStateRef.current.selectedLayout && updatedLayout.id === layoutStateRef.current.selectedLayout.id diff --git a/packages/suite-base/src/services/ILayoutManager.ts b/packages/suite-base/src/services/ILayoutManager.ts index 95754ea..c6adfb8 100644 --- a/packages/suite-base/src/services/ILayoutManager.ts +++ b/packages/suite-base/src/services/ILayoutManager.ts @@ -10,7 +10,7 @@ import { Layout, LayoutPermission } from "@lichtblick/suite-base/services/ILayou export type LayoutManagerChangeEvent = | { type: "delete"; updatedLayout?: undefined; layoutId: LayoutID } - | { type: "change"; updatedLayout: Layout | undefined }; + | { type: "change" | "revert"; updatedLayout: Layout | undefined }; export type LayoutManagerEventTypes = { /** diff --git a/packages/suite-base/src/services/LayoutManager/LayoutManager.ts b/packages/suite-base/src/services/LayoutManager/LayoutManager.ts index 1236841..475140d 100644 --- a/packages/suite-base/src/services/LayoutManager/LayoutManager.ts +++ b/packages/suite-base/src/services/LayoutManager/LayoutManager.ts @@ -445,7 +445,7 @@ export default class LayoutManager implements ILayoutManager { working: undefined, }); }); - this.#notifyChangeListeners({ type: "change", updatedLayout: result }); + this.#notifyChangeListeners({ type: "revert", updatedLayout: result }); return result; }