Skip to content

perf: avoid unnecessary operation for improve performance#698

Merged
mcollina merged 7 commits intofastify:masterfrom
cesco69:perf-cast
Apr 19, 2024
Merged

perf: avoid unnecessary operation for improve performance#698
mcollina merged 7 commits intofastify:masterfrom
cesco69:perf-cast

Conversation

@cesco69
Copy link
Contributor

@cesco69 cesco69 commented Mar 19, 2024 •
edited
Loading

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

ivan-tymoshenko
ivan-tymoshenko requested changes Mar 19, 2024
View reviewed changes
@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

gurgunday commented Apr 11, 2024

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 •
edited
Loading

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]>

gurgunday
gurgunday requested changes Apr 11, 2024
View reviewed changes
Copy link
Member

@gurgunday gurgunday left a comment •
edited
Loading

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
@cesco69
remove switch
16c54b9 
Signed-off-by: francesco <francesco.bagnoli.69@gmail.com>
@cesco69
fix format
ab473e9 
Signed-off-by: francesco <francesco.bagnoli.69@gmail.com>
@cesco69
fix lint
f01f627 
Signed-off-by: francesco <francesco.bagnoli.69@gmail.com>
@cesco69
Copy link
Contributor Author

cesco69 commented Apr 11, 2024 •
edited
Loading

@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
mcollina
mcollina approved these changes Apr 12, 2024
View reviewed changes
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

mcollina commented Apr 12, 2024

@ivan-tymoshenko ptal

ivan-tymoshenko
ivan-tymoshenko approved these changes Apr 12, 2024
View reviewed changes
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
@cesco69 cesco69 deleted the perf-cast branch April 19, 2024 12:03
@cesco69 cesco69 mentioned this pull request Apr 23, 2024
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Uzlopak Uzlopak Uzlopak left review comments

@mcollina mcollina mcollina approved these changes

@ivan-tymoshenko ivan-tymoshenko ivan-tymoshenko approved these changes

@gurgunday gurgunday gurgunday approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@cesco69 @Uzlopak @gurgunday @mcollina @ivan-tymoshenko