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

perf: avoid unnecessary operation for improve performance #698

Merged
merged 7 commits into from
Apr 19, 2024

Conversation

cesco69
Copy link
Contributor

@cesco69 cesco69 commented Mar 19, 2024

This PR has some microptimization for reduce the overhead of function call that can replaced with more faster inline control

Side note: the code become a bit ugly but seems more faster

PS. someone can run benchmark on github (master vs PR)? I have noticed my PC is a bit unstable on benchmark results

Master

short string............................................. x 21,651,330 ops/sec ±1.82% (185 runs sampled)
unsafe short string...................................... x 890,279,050 ops/sec ±1.42% (181 runs sampled)
short string with double quote........................... x 12,229,862 ops/sec ±2.59% (190 runs sampled)
long string without double quotes........................ x 15,138 ops/sec ±0.37% (190 runs sampled)
unsafe long string without double quotes................. x 1,022,578,589 ops/sec ±0.39% (186 runs sampled)
long string.............................................. x 15,864 ops/sec ±0.47% (189 runs sampled)
unsafe long string....................................... x 1,054,094,454 ops/sec ±0.47% (190 runs sampled)
number................................................... x 1,060,238,499 ops/sec ±0.43% (190 runs sampled)
integer.................................................. x 219,863,056 ops/sec ±0.41% (189 runs sampled)

PR

short string............................................. x 23,801,382 ops/sec ±0.55% (189 runs sampled)
unsafe short string...................................... x 1,049,266,290 ops/sec ±0.35% (191 runs sampled)
short string with double quote........................... x 12,629,236 ops/sec ±0.43% (191 runs sampled)
long string without double quotes........................ x 15,461 ops/sec ±0.36% (190 runs sampled)
unsafe long string without double quotes................. x 1,060,363,294 ops/sec ±0.41% (191 runs sampled)
long string.............................................. x 16,106 ops/sec ±0.35% (191 runs sampled)
unsafe long string....................................... x 1,047,641,170 ops/sec ±0.37% (191 runs sampled)
number................................................... x 1,075,211,555 ops/sec ±0.43% (191 runs sampled)
integer.................................................. x 1,050,029,012 ops/sec ±0.44% (189 runs sampled)

Checklist

test/fix-604.test.js Outdated Show resolved Hide resolved
@cesco69 cesco69 requested a review from ivan-tymoshenko March 19, 2024 13:42
@cesco69 cesco69 closed this Apr 11, 2024
@Uzlopak
Copy link
Contributor

Uzlopak commented Apr 11, 2024

@cesco69

Why closed?

@cesco69
Copy link
Contributor Author

cesco69 commented Apr 11, 2024

@Uzlopak I thought you don't want to merge anymore :D

@cesco69 cesco69 reopened this Apr 11, 2024
@gurgunday
Copy link
Member

I want to check the V8 bytecode to see if a switch typeof uses string comparison or not, typeof exp === "{type}" doesn't but switch might

@Uzlopak
Copy link
Contributor

Uzlopak commented Apr 11, 2024

The problem is, that we have always maendering interests, so it is kind of hard to always get things done in a timely manner. So just dont think we are not interested, we are just busy, like you ;).

@gurgunday
Copy link
Member

gurgunday commented Apr 11, 2024

Seems like I cannot get the TestTypeOf bytecode without using typeof exp ===; we instead get the good ol' TestEqualStrict bytecode for each case, which means that string comparison is taking place, there is no way that it can be faster

const a = (b) => {
  if (typeof b === "function") {
    return b;
  }
};
[generated bytecode for function: a (0x328e80b5b9f1 <SharedFunctionInfo a>)]
Bytecode length: 11
Parameter count 2
Register count 0
Frame size 0
Bytecode age: 0
 3174 S> 0x328e80b5ccae @    0 : 0b 03             Ldar a0
         0x328e80b5ccb0 @    2 : 20 06             TestTypeOf #6
         0x328e80b5ccb2 @    4 : 99 05             JumpIfFalse [5] (0x328e80b5ccb7 @ 9)
 3209 S> 0x328e80b5ccb4 @    6 : 0b 03             Ldar a0
 3218 S> 0x328e80b5ccb6 @    8 : a9                Return
         0x328e80b5ccb7 @    9 : 0e                LdaUndefined
 3223 S> 0x328e80b5ccb8 @   10 : a9                Return
Constant pool (size = 0)
Handler Table (size = 0)
Source Position Table (size = 12)
0x328e80b5ccc1 <ByteArray[12]>
const a = (b) => {
  switch (typeof b) {
    case "function":
      return b;
  }
};
[generated bytecode for function: a (0x1ded2941b9f1 <SharedFunctionInfo a>)]
Bytecode length: 18
Parameter count 2
Register count 1
Frame size 8
Bytecode age: 0
 3174 S> 0x1ded2941ccc6 @    0 : 0b 03             Ldar a0
         0x1ded2941ccc8 @    2 : 56                TypeOf
         0x1ded2941ccc9 @    3 : c4                Star0
         0x1ded2941ccca @    4 : 13 00             LdaConstant [0]
         0x1ded2941cccc @    6 : 6c fa 00          TestEqualStrict r0, [0]
         0x1ded2941cccf @    9 : 98 04             JumpIfTrue [4] (0x1ded2941ccd3 @ 13)
         0x1ded2941ccd1 @   11 : 8a 05             Jump [5] (0x1ded2941ccd6 @ 16)
 3221 S> 0x1ded2941ccd3 @   13 : 0b 03             Ldar a0
 3230 S> 0x1ded2941ccd5 @   15 : a9                Return
         0x1ded2941ccd6 @   16 : 0e                LdaUndefined
 3235 S> 0x1ded2941ccd7 @   17 : a9                Return
Constant pool (size = 1)
0x1ded2941cc79: [FixedArray] in OldSpace
 - map: 0x0624cc240211 <Map(FIXED_ARRAY_TYPE)>
 - length: 1
           0: 0x0624cc246351 <String[8]: #function>
Handler Table (size = 0)
Source Position Table (size = 12)
0x1ded2941ccd9 <ByteArray[12]>

Copy link
Member

@gurgunday gurgunday left a comment

Choose a reason for hiding this comment

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

#698 (comment)

The rest of the PR (Number -> +, isFinite -> !== Infinity || !== -Infinity) LGTM

cesco69 added 3 commits April 11, 2024 15:14
Signed-off-by: francesco <[email protected]>
Signed-off-by: francesco <[email protected]>
Signed-off-by: francesco <[email protected]>
@cesco69
Copy link
Contributor Author

cesco69 commented Apr 11, 2024

@gurgunday done
PR

short string............................................. x 23,800,051 ops/sec ±1.32% (188 runs sampled)
unsafe short string...................................... x 1,058,117,728 ops/sec ±0.41% (191 runs sampled)
short string with double quote........................... x 12,756,936 ops/sec ±0.47% (189 runs sampled)
long string without double quotes........................ x 15,517 ops/sec ±0.43% (191 runs sampled)
unsafe long string without double quotes................. x 1,060,660,716 ops/sec ±0.48% (190 runs sampled)
long string.............................................. x 16,018 ops/sec ±0.44% (191 runs sampled)
unsafe long string....................................... x 1,057,375,214 ops/sec ±0.45% (190 runs sampled)
number................................................... x 1,064,004,713 ops/sec ±0.47% (188 runs sampled)
integer.................................................. x 220,353,556 ops/sec ±0.47% (190 runs sampled)

MASTER

short string............................................. x 22,543,936 ops/sec ±0.39% (193 runs sampled)
unsafe short string...................................... x 1,048,134,640 ops/sec ±0.41% (190 runs sampled)
short string with double quote........................... x 13,623,011 ops/sec ±0.49% (190 runs sampled)
long string without double quotes........................ x 15,501 ops/sec ±0.46% (190 runs sampled)
unsafe long string without double quotes................. x 1,061,984,142 ops/sec ±0.44% (191 runs sampled)
long string.............................................. x 16,102 ops/sec ±0.47% (191 runs sampled)
unsafe long string....................................... x 1,059,719,472 ops/sec ±0.48% (190 runs sampled)
number................................................... x 1,061,655,705 ops/sec ±0.46% (187 runs sampled)
integer.................................................. x 220,281,194 ops/sec ±0.42% (190 runs sampled)

@cesco69 cesco69 requested a review from gurgunday April 11, 2024 13:25
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina
Copy link
Member

@ivan-tymoshenko ptal

Copy link
Member

@ivan-tymoshenko ivan-tymoshenko left a comment

Choose a reason for hiding this comment

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

lgtm

@cesco69
Copy link
Contributor Author

cesco69 commented Apr 19, 2024

@mcollina seem all "ok" for the merge!

@mcollina mcollina merged commit ed47c66 into fastify:master Apr 19, 2024
19 checks passed
@cesco69 cesco69 deleted the perf-cast branch April 19, 2024 12:03
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.

5 participants