Skip to content

bugfix: performance.now function should return MS instead of nano - #428

Merged
Jarred-Sumner merged 2 commits into
oven-sh:mainfrom
Pruxis:pruxis/bugfix-perf-now
Jul 10, 2022
Merged

Jarred-Sumner merged 2 commits into
oven-sh:mainfrom
Pruxis:pruxis/bugfix-perf-now

Conversation

@Pruxis

@Pruxis Pruxis commented Jul 8, 2022

Copy link
Copy Markdown
Contributor

No description provided.

@Pruxis Pruxis left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comment thread src/bun.js/bindings/ZigGlobalObject.cpp Outdated
Comment on lines 1499 to 1500
double result = time / 1000000000000000.0;
return JSValue::encode(jsNumber(time));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You are still returning the time variable. Please calculate double result = time / 1000000.0; and change jsNumber(time) to jsNumber(result)

@Pruxis Pruxis Jul 8, 2022 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I overlooked that, the added 0's are incorrect and should be time / 1_000_000.0 I suppose

@afrokick

afrokick commented Jul 9, 2022 •

Copy link
Copy Markdown
Contributor

Maybe should add a test like expect(Math.abs(performance.now() - Date.now())).toBeCloseTo(0, 3);?

https://github.com/Jarred-Sumner/bun/blob/main/test/bun.js/performance.test.js

@Jarred-Sumner
Jarred-Sumner merged commit 92225fa into oven-sh:main Jul 10, 2022
@dmjio dmjio mentioned this pull request Apr 2, 2025
dylan-conway added a commit that referenced this pull request Aug 14, 2026
…8 fix); test it via a forced C.UTF-8 locale on Linux; adapt netd's Job to the new JobContext::run signature

WEBKIT_VERSION points at the preview tag until #428 merges; the Linux-run
intl test fails on the previous WebKit (en-US-u-va-posix) and passes with it.
dylan-conway added a commit that referenced this pull request Aug 14, 2026
…reats C.UTF-8 as the C locale)

Replaces the preview pin; #428 is the only commit between the previous pin
(687eb8e1b73c) and this one.
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.

4 participants