Skip to content

feat(tools-performance): Add @rnx-kit/tools-performance package#4079

Open
JasonVMo wants to merge 10 commits intomainfrom
user/jasonvmo/tools-perf
Open

feat(tools-performance): Add @rnx-kit/tools-performance package#4079
JasonVMo wants to merge 10 commits intomainfrom
user/jasonvmo/tools-perf

Conversation

@JasonVMo
Copy link
Copy Markdown
Collaborator

@JasonVMo JasonVMo commented Apr 9, 2026

Description

This adds a framework for doing lightweight performance tracing, some basic reporting, and enabling tracing at a global level.

Usage

In packages, to add instrumentation:

import { getTrace } from "@rnx-kit/tools-performance";

async function myFunction() {
  // will return a logging tracer if "transform" is enabled, a passthrough if not
  const trace = getTrace("transform");
  // trace a sync closure
  const result1 = trace("func1", () => doSomething(p1, p2));
  // trace a sync function without creating a closure
  const result2 = trace("process result, doSomethingElse, result1, options);
  // trace an async function and return
  const final = await trace("call async", finalCall, result2);
  return final;
}

Right now, to enable this for metro bundling a user would add in their metro.config.js:

import { trackPerformance } from "@rnx-kit/tools-performance";

// these would all work
trackPerformance("transform"); // only turn on this one area
trackPerformance(["transform", "metro"]); // turn on a set of areas
trackPerformance(/* or true */); // turn on all performance tracing

Future things to add (or consider)

  • instrument various parts of our infrastructure
  • add an implementation of unstable_perfLoggerFactory that hooks into this
  • have our metro config automatically add the unstable_perfLoggerFactory if "metro" is enabled
  • consider adding a cli option that will enable perf logging for our tools automatically

Other notes

  • text is styled using the formatting from node:utils
  • simple table formatting in the same format as console.table is added, except with support for formatted text.

Comment on lines +73 to +77
} else {
for (const cat of category) {
this.enabled.add(cat);
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably also check that category is an actual array:

Suggested change
} else {
for (const cat of category) {
this.enabled.add(cat);
}
}
} else if (Array.isArray(category)) {
for (const cat of category) {
this.enabled.add(cat);
}
} else {
throw new Error(`invalid category: ${category}`);
}

Copy link
Copy Markdown
Member

@tido64 tido64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly minor issues/nits

@@ -0,0 +1,221 @@
import { styleText } from "util";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import { styleText } from "util";
import { styleText } from "node:util";

Comment on lines +43 to +45
if (this.exitHandlers) {
this.exitHandlers.delete(callback);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (this.exitHandlers) {
this.exitHandlers.delete(callback);
}
this.exitHandlers?.delete(callback);

*/
stop(processExit = false) {
// first attempt to close the initial start event if it exists
if (this.startVal !== undefined) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This supports both null/undefined and is shorter:

Suggested change
if (this.startVal !== undefined) {
if (this.startVal != null) {

private timingRecorder(tag: string, startTime?: number) {
const timeNow = performance.now();
tag = this.coerceTag(tag);
if (startTime === undefined) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (startTime === undefined) {
if (startTime == null) {


private markingRecorder(tag: string, startMark?: string) {
tag = this.coerceTag(tag);
if (startMark === undefined) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (startMark === undefined) {
if (startMark == null) {

if (intlFormatter) {
text = intlFormatter.format(value);
} else {
text = digits !== undefined ? value.toFixed(digits) : String(value);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
text = digits !== undefined ? value.toFixed(digits) : String(value);
text = digits != null ? value.toFixed(digits) : String(value);

text ??= String(value);
key = text;
}
if (maxWidth !== undefined && text.length > maxWidth) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (maxWidth !== undefined && text.length > maxWidth) {
if (maxWidth != null && text.length > maxWidth) {

completions: 0,
total: 0,
};
if (duration !== undefined) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (duration !== undefined) {
if (duration != null) {

if (reportSort && reportSort.length > 0) {
for (const sortCol of reportSort) {
const index = cols.indexOf(sortCol);
if (index !== -1) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure positive index:

Suggested change
if (index !== -1) {
if (index >= 0) {

Comment on lines +167 to +171
const rows = Array.from(this.timings.values()).map(
(entry: OperationData) => {
return cols.map((col: PerfReportColumn) => getCellValue(entry, col));
}
);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use .forEach instead of Array.from to avoid the temporary array?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants