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

Uses WTF::String::Number instead of WTF::String::format in CSSPrimitiveValue::CustomCSSText. #15830

Closed
wants to merge 1 commit into from

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Mar 14, 2019

CSSPrimitiveValue::CustomCSSText can be called huge amount of times
through InspectorDOMSnapshotAgent::getSnapshot. It underlyingly calls
WTF::String::format which is sort of expensive by considering the huge
call volume.

Actually CustomCSSText only needs to convert number to string. And

WTF::String provides a cheaper way, String::Number, to achieve same
goal.

WTF::String uses WTF Dtoa function to mimic "%.[precision]g" format. According to C11 standard
(http://port70.net/~nsz/c/c11/n1570.html#7.21.6), when using "%[.percisiong]g" format,

(1) The exponent always contains at least two digits, and only as many more digits as
necessary to represent the exponent. If the value is zero, the exponent is zero.
(2) Finally, unless the # flag is used, any trailing zeros are removed from the
fractional portion of the result and the decimal-point character is removed if there
is no fractional portion remaining.

But the WTF Dota function doesn't comply with the rules. Fix it in this change.

Change-Id: I896981f54aa0e327c85d6bc4cb87dd9a0291e124
Reviewed-on: https://chromium-review.googlesource.com/1512138
WPT-Export-Revision: ad2ac74a96a48d2f22205190b48a0f66775ad7d9

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

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

Already reviewed downstream.

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-1512138 branch 5 times, most recently from 5c415d5 to 59b58d0 Compare March 14, 2019 23:41
…veValue::CustomCSSText.

CSSPrimitiveValue::CustomCSSText can be called huge amount of times
through InspectorDOMSnapshotAgent::getSnapshot. It underlyingly calls
WTF::String::format which is sort of expensive by considering the huge
call volume.

Actually CustomCSSText only needs to convert number to string. And

WTF::String provides a cheaper way, String::Number, to achieve same
goal.

WTF::String uses WTF Dtoa function to mimic "%.[precision]g" format. According to C11 standard
(http://port70.net/~nsz/c/c11/n1570.html#7.21.6), when using "%[.percisiong]g" format,

(1) The exponent always contains at least two digits, and only as many more digits as
necessary to represent the exponent. If the value is zero, the exponent is zero.
(2) Finally, unless the # flag is used, any trailing zeros are removed from the
fractional portion of the result and the decimal-point character is removed if there
is no fractional portion remaining.

But the WTF Dota function doesn't comply with the rules. Fix it in this change.

Change-Id: I896981f54aa0e327c85d6bc4cb87dd9a0291e124
@KyleJu
Copy link
Contributor

KyleJu commented May 10, 2019

Close this PR because the Chromium CL does not have exportable changes.

@KyleJu KyleJu closed this May 10, 2019
@KyleJu KyleJu deleted the chromium-export-cl-1512138 branch May 10, 2019 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants