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.

Sign up for GitHub

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

Jump to bottom

feat: improve performance on Node.js 16 by disabling class property definitions #1581

Merged
merged 2 commits into from
Oct 8, 2023
Merged

feat: improve performance on Node.js 16 by disabling class property definitions #1581

merged 2 commits into from
Oct 8, 2023

Conversation

MichaelSun90
Copy link
Contributor

add declare for class level properties regard to #1579

@codecov
Copy link

codecov bot commented Oct 5, 2023 •
edited
Loading

Codecov Report

Merging #1581 (aaef4bf) into master (f4d91f1) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1581   +/-   ##
=======================================
  Coverage   79.78%   79.78%           
=======================================
  Files          92       92           
  Lines        4709     4709           
  Branches      871      871           
=======================================
  Hits         3757     3757           
  Misses        684      684           
  Partials      268      268
Files Coverage Δ
src/bulk-load-payload.ts 60.00% <ø> (ø)
src/bulk-load.ts 85.43% <ø> (ø)
src/collation.ts 100.00% <ø> (ø)
src/connection.ts 62.27% <ø> (ø)
src/debug.ts 100.00% <ø> (ø)
src/errors.ts 100.00% <ø> (ø)
src/errors/abort-error.ts 100.00% <ø> (ø)
src/errors/timeout-error.ts 100.00% <ø> (ø)
src/incoming-message-stream.ts 97.56% <ø> (ø)
src/login7-payload.ts 89.23% <ø> (ø)
... and 15 more

@arthurschreiber arthurschreiber changed the title chore: add declare for class properties feat: don't use class properties for class definitions Oct 6, 2023
@arthurschreiber
Copy link
Collaborator

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-7.html#the-usedefineforclassfields-flag-and-the-declare-property-modifier has some more information around this, and the declare keyword on class properties. In a nutshell, declare just means "don't include this in the final JavaScript code", which is exactly what we want here. 👍

@arthurschreiber arthurschreiber changed the title feat: don't use class properties for class definitions feat: don't emit class properties for class definitions Oct 6, 2023
@arthurschreiber
Copy link
Collaborator

arthurschreiber commented Oct 6, 2023 •
edited
Loading

@MichaelSun90 Do you have the time to run a few of the benchmarks before / after this change and document the difference here in this pull request body? 🙇‍♂️

@MichaelSun90
Copy link
Contributor Author

MichaelSun90 commented Oct 6, 2023 •
edited by arthurschreiber
Loading

Here are the benchmark compression (rpcrequest-payload-tvp.js is not in here, run into a "No collation was set by the server for the current connection. Error"):

Without the change:

node benchmarks/token-parser/done-token.js

token-parser\done-token.js tokenCount=10 n=10: 1,668.58554003771 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=100 n=10: 1,059.6363328105795 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=1000 n=10: 171.59869928185944 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=10000 n=10: 55.31775069174847 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=10 n=100: 8,287.544649146797 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=100 n=100: 2,125.186219442479 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=1000 n=100: 580.618939789816 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=10000 n=100: 70.85470901813228 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=10 n=1000: 16,866.364421416234 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=100 n=1000: 6,859.406853370575 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=1000 n=1000: 795.0260625443824 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=10000 n=1000: 52.691578848202866 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)

With the change:

node benchmarks/token-parser/done-token.js

token-parser\done-token.js tokenCount=10 n=10: 3,651.3674371051957 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=100 n=10: 1,587.9317189360856 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=1000 n=10: 326.9501760626698 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=10000 n=10: 83.18733955241883 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=10 n=100: 18,170.255292086855 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=100 n=100: 3,415.1372543662533 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=1000 n=100: 1,091.23368332835 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=10000 n=100: 148.52021879998634 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=10 n=1000: 32,131.301349835972 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=100 n=1000: 9,961.478960858361 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=1000 n=1000: 1,271.9241566888456 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\done-token.js tokenCount=10000 n=1000: 91.4727610308329 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)

Without the change:

node benchmarks\token-parser\simple-tokens.js

token-parser\simple-tokens.js n=10: 441.47966323931286 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\simple-tokens.js n=100: 804.1261320085623 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\simple-tokens.js n=1000: 2,830.196155235127 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)

With the change:

node benchmarks\token-parser\simple-tokens.js

token-parser\simple-tokens.js n=10: 315.70639305445934 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\simple-tokens.js n=100: 1,109.6599225235443 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\simple-tokens.js n=1000: 2,427.9641253732693 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)

Without the change:

node benchmarks\token-parser\colmetadata-token.js                                       

token-parser\colmetadata-token.js tokenCount=10 n=10: 1,939.9771082701225 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=100 n=10: 304.9347592082674 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=1000 n=10: 101.16215078825547 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=10000 n=10: 21.673314233363943 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=10 n=100: 3,381.8058843422386 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=100 n=100: 915.3879459884496 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=1000 n=100: 172.5908177197612 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=10000 n=100: 18.250732616346003 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=10 n=1000: 8,509.399908609044 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=100 n=1000: 1,365.1616214843675 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=1000 n=1000: 177.76676729924853 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=10000 n=1000: 18.287694924585487 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)

With the change:

node benchmarks\token-parser\colmetadata-token.js

token-parser\colmetadata-token.js tokenCount=10 n=10: 1,665.4730776277001 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=100 n=10: 207.11008936800357 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=1000 n=10: 97.74540449980745 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=10000 n=10: 19.57565078761652 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=10 n=100: 2,868.3375689118097 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=100 n=100: 1,339.6062629272003 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=1000 n=100: 258.0316878394568 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=10000 n=100: 23.023730466897312 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=10 n=1000: 6,431.178352653665 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=100 n=1000: 1,890.07262037022 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=1000 n=1000: 219.38780009892633 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
token-parser\colmetadata-token.js tokenCount=10000 n=1000: 20.085217440043312 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)

Without the change:

node benchmarks\query\call-tvp.js

query\call-tvp.js n=10: 6.445395383756263 (minor: 14 - 8.704899787902832ms, major: 1 - 2.6089999675750732ms, incremental: 2 - 0.34509992599487305ms)
query\call-tvp.js n=100: 7.548510312812532 (minor: 134 - 75.49629855155945ms, major: 1 - 2.4422998428344727ms, incremental: 2 - 0.3512001037597656ms)

With the change:

node benchmarks\query\call-tvp.js

query\call-tvp.js n=10: 8.31124688733415 (minor: 14 - 9.791000366210938ms, major: 1 - 2.15310001373291ms, incremental: 2 - 0.2853999137878418ms)
query\call-tvp.js n=100: 10.899422526795686 (minor: 134 - 63.37340188026428ms, major: 1 - 2.1798999309539795ms, incremental: 2 - 0.34960007667541504ms)

Without the change:

node benchmarks\query\insert-varbinary.js

query\insert-varbinary.js size=10 n=10: 323.99472536587103 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\insert-varbinary.js size=1024 n=10: 316.8477451530216 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\insert-varbinary.js size=1048576 n=10: 47.82002836684082 (minor: 1 - 0.47880005836486816ms, major: 1 - 2.5920000076293945ms, incremental: 2 - 0.3377997875213623ms)    
query\insert-varbinary.js size=10485760 n=10: 8.166817371669898 (minor: 9 - 8.744000434875488ms, major: 1 - 2.7183001041412354ms, incremental: 1 - 0.0017001628875732422ms)  
query\insert-varbinary.js size=10 n=100: 716.0446267653185 (minor: 1 - 1.2450001239776611ms, major: 1 - 3.699399948120117ms, incremental: 2 - 0.32629990577697754ms)
query\insert-varbinary.js size=1024 n=100: 472.82173391294975 (minor: 1 - 1.8265001773834229ms, major: 1 - 4.29200005531311ms, incremental: 2 - 0.6583001613616943ms)        
query\insert-varbinary.js size=1048576 n=100: 42.14043400770108 (minor: 8 - 7.432199954986572ms, major: 1 - 5.532299995422363ms, incremental: 2 - 0.5611000061035156ms)      
query\insert-varbinary.js size=10485760 n=100: 5.678108860419167 (minor: 13 - 19.336599826812744ms, major: 86 - 292.33879947662354ms, incremental: 171 - 37.725998640060425ms)

With the change:

node benchmarks\query\insert-varbinary.js

query\insert-varbinary.js size=10 n=10: 349.62345553838514 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\insert-varbinary.js size=1024 n=10: 446.72372817754587 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\insert-varbinary.js size=1048576 n=10: 35.623090602343716 (minor: 1 - 0.5211000442504883ms, major: 1 - 4.847699880599976ms, incremental: 2 - 0.4124002456665039ms)
query\insert-varbinary.js size=10485760 n=10: 6.779042860159531 (minor: 8 - 7.832900524139404ms, major: 1 - 4.336199998855591ms, incremental: 1 - 0.0019998550415039062ms)
query\insert-varbinary.js size=10 n=100: 944.2710133349952 (minor: 1 - 1.311000108718872ms, major: 1 - 2.499199867248535ms, incremental: 2 - 0.34870004653930664ms)
query\insert-varbinary.js size=1024 n=100: 913.1694559427698 (minor: 1 - 1.2991998195648193ms, major: 1 - 2.695500135421753ms, incremental: 2 - 0.2961997985839844ms)
query\insert-varbinary.js size=1048576 n=100: 48.0479072229506 (minor: 8 - 6.3902997970581055ms, major: 1 - 3.1403000354766846ms, incremental: 2 - 0.4375ms)
query\insert-varbinary.js size=10485760 n=100: 6.812765282693141 (minor: 6 - 7.343799829483032ms, major: 96 - 297.36200046539307ms, incremental: 191 - 41.44260048866272ms)

Without the change:

node benchmarks\query\select-many-rows.js

query\select-many-rows.js size=10 n=10: 523.5163546509193 (minor: 0 - 0ms, major: 1 - 2.288800001144409ms, incremental: 1 - 0.0017998218536376953ms)
query\select-many-rows.js size=100 n=10: 231.80558002392235 (minor: 1 - 0.8533999919891357ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-many-rows.js size=1000 n=10: 104.93575832875113 (minor: 3 - 2.6902999877929688ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-many-rows.js size=10000 n=10: 13.836278362122247 (minor: 27 - 13.734999895095825ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-many-rows.js size=10 n=100: 515.727363823479 (minor: 2 - 2.7436001300811768ms, major: 1 - 3.9463000297546387ms, incremental: 2 - 0.40829968452453613ms)
query\select-many-rows.js size=100 n=100: 340.25120746647247 (minor: 8 - 8.370699644088745ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-many-rows.js size=1000 n=100: 112.62400184162767 (minor: 30 - 23.191100120544434ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-many-rows.js size=10000 n=100: 14.363910594194188 (minor: 272 - 131.80780148506165ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-many-rows.js size=10 n=1000: 1,094.7842289763112 (minor: 13 - 20.27910017967224ms, major: 1 - 3.0546000003814697ms, incremental: 2 - 0.42479991912841797ms)
query\select-many-rows.js size=100 n=1000: 627.1110124456452 (minor: 48 - 50.87429881095886ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-many-rows.js size=1000 n=1000: 152.75844322648442 (minor: 294 - 136.6864984035492ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-many-rows.js size=10000 n=1000: 17.484064608269488 (minor: 2712 - 1173.439299583435ms, major: 4 - 19.424699783325195ms, incremental: 8 - 2.3496994972229004ms)

With the change:

node benchmarks\query\select-many-rows.js

query\select-many-rows.js size=10 n=10: 410.7872738102574 (minor: 0 - 0ms, major: 1 - 3.252700090408325ms, incremental: 2 - 0.40960001945495605ms)
query\select-many-rows.js size=100 n=10: 259.82056791579737 (minor: 1 - 0.625ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-many-rows.js size=1000 n=10: 104.7155506781379 (minor: 3 - 2.584399938583374ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-many-rows.js size=10000 n=10: 11.501657561379458 (minor: 27 - 13.026598930358887ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-many-rows.js size=10 n=100: 588.7811288582827 (minor: 2 - 2.3436999320983887ms, major: 1 - 3.188199996948242ms, incremental: 2 - 0.39550018310546875ms)
query\select-many-rows.js size=100 n=100: 468.6373805794795 (minor: 7 - 8.864399909973145ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-many-rows.js size=1000 n=100: 129.50371844026756 (minor: 29 - 18.684200286865234ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-many-rows.js size=10000 n=100: 12.770236236727847 (minor: 271 - 140.66659784317017ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-many-rows.js size=10 n=1000: 980.9205075753547 (minor: 12 - 21.993500232696533ms, major: 1 - 4.662299871444702ms, incremental: 2 - 0.45980000495910645ms)
query\select-many-rows.js size=100 n=1000: 676.6031706165858 (minor: 49 - 48.812599658966064ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-many-rows.js size=1000 n=1000: 112.27403433131138 (minor: 294 - 155.9489986896515ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-many-rows.js size=10000 n=1000: 14.246847745317242 (minor: 2713 - 1237.656598329544ms, major: 3 - 14.448100090026855ms, incremental: 6 - 1.6974997520446777ms)

Without the change:

node benchmarks\query\select-nvarchar.js

query\select-nvarchar.js size=10 n=10: 598.8848763602173 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-nvarchar.js size=100 n=10: 481.09535791089155 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-nvarchar.js size=1000 n=10: 683.046112443051 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-nvarchar.js size=10000 n=10: 469.32051775439515 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 1 - 0.3636000156402588ms)
query\select-nvarchar.js size=10 n=100: 1,168.0728316772008 (minor: 1 - 1.3055999279022217ms, major: 1 - 3.0534000396728516ms, incremental: 2 - 0.3539998531341553ms)
query\select-nvarchar.js size=100 n=100: 673.3435412214183 (minor: 1 - 1.7330999374389648ms, major: 1 - 3.297299861907959ms, incremental: 2 - 0.46900010108947754ms)
query\select-nvarchar.js size=1000 n=100: 1,295.9108828004114 (minor: 1 - 1.246999979019165ms, major: 1 - 2.3824000358581543ms, incremental: 2 - 0.2925999164581299ms)
query\select-nvarchar.js size=10000 n=100: 910.768378850501 (minor: 2 - 2.424299955368042ms, major: 1 - 2.441999912261963ms, incremental: 2 - 0.3347001075744629ms)
query\select-nvarchar.js size=10 n=1000: 1,635.3521264729004 (minor: 7 - 11.077400207519531ms, major: 1 - 2.5434999465942383ms, incremental: 2 - 0.26619982719421387ms)
query\select-nvarchar.js size=100 n=1000: 1,799.03711935298 (minor: 7 - 10.036500215530396ms, major: 1 - 2.443000078201294ms, incremental: 2 - 0.2941000461578369ms)
query\select-nvarchar.js size=1000 n=1000: 1,583.0229025421447 (minor: 7 - 11.015199899673462ms, major: 1 - 2.7639000415802ms, incremental: 2 - 0.2542998790740967ms)
query\select-nvarchar.js size=10000 n=1000: 1,168.0810180994154 (minor: 9 - 14.760599613189697ms, major: 1 - 3.2321999073028564ms, incremental: 2 - 0.32289981842041016ms

With the change:

node benchmarks\query\select-nvarchar.js

query\select-nvarchar.js size=10 n=10: 480.125216656504 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-nvarchar.js size=100 n=10: 609.9680986684397 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-nvarchar.js size=1000 n=10: 784.3937028873532 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-nvarchar.js size=10000 n=10: 270.07975455151904 (minor: 1 - 1.369999885559082ms, major: 0 - 0ms, incremental: 2 - 0.5931999683380127ms)
query\select-nvarchar.js size=10 n=100: 1,238.5772215740335 (minor: 1 - 1.1556000709533691ms, major: 1 - 2.6440000534057617ms, incremental: 2 - 0.2849998474121094ms)
query\select-nvarchar.js size=100 n=100: 979.1039632170224 (minor: 1 - 1.1222000122070312ms, major: 1 - 2.47160005569458ms, incremental: 2 - 0.3187999725341797ms)
query\select-nvarchar.js size=1000 n=100: 483.488388059384 (minor: 1 - 1.486799955368042ms, major: 1 - 2.922300100326538ms, incremental: 2 - 0.4119999408721924ms)
query\select-nvarchar.js size=10000 n=100: 891.8713065379518 (minor: 1 - 0.7262001037597656ms, major: 1 - 3.802799940109253ms, incremental: 2 - 0.34800004959106445ms)
query\select-nvarchar.js size=10 n=1000: 1,746.5885196387285 (minor: 7 - 10.355600118637085ms, major: 1 - 2.860599994659424ms, incremental: 2 - 0.31819987297058105ms)
query\select-nvarchar.js size=100 n=1000: 1,272.2923106851306 (minor: 7 - 14.37529969215393ms, major: 1 - 3.0332000255584717ms, incremental: 2 - 0.42480015754699707ms)
query\select-nvarchar.js size=1000 n=1000: 940.7468702292375 (minor: 7 - 15.719500064849854ms, major: 1 - 4.465800046920776ms, incremental: 2 - 0.5539000034332275ms)
query\select-nvarchar.js size=10000 n=1000: 867.8129144620242 (minor: 9 - 16.23609972000122ms, major: 1 - 3.4093000888824463ms, incremental: 2 - 0.40649986267089844ms)

Without the change:

node benchmarks\query\select-varbinary.js

query\select-varbinary.js size=10 n=10: 572.6196202386678 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-varbinary.js size=100 n=10: 593.1549913992527 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-varbinary.js size=1000 n=10: 698.1728815689341 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-varbinary.js size=10000 n=10: 374.9685963800532 (minor: 1 - 1.223599910736084ms, major: 1 - 2.835599899291992ms, incremental: 2 - 0.42350006103515625ms)
query\select-varbinary.js size=1048576 n=10: 59.343342181425655 (minor: 1 - 0.5796000957489014ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-varbinary.js size=10485760 n=10: 9.672466116142138 (minor: 13 - 8.917999982833862ms, major: 4 - 15.33240008354187ms, incremental: 10 - 2.108199119567871ms)
query\select-varbinary.js size=10 n=100: 1,134.2600994519255 (minor: 1 - 1.455399990081787ms, major: 1 - 2.9296000003814697ms, incremental: 2 - 0.32829999923706055ms)
query\select-varbinary.js size=100 n=100: 1,048.7500996312594 (minor: 1 - 1.3910999298095703ms, major: 1 - 2.95740008354187ms, incremental: 2 - 0.34940004348754883ms)
query\select-varbinary.js size=1000 n=100: 873.5264700358583 (minor: 1 - 1.4872000217437744ms, major: 1 - 3.128700017929077ms, incremental: 2 - 0.37889981269836426ms)
query\select-varbinary.js size=10000 n=100: 798.6958893518662 (minor: 1 - 1.6521000862121582ms, major: 1 - 3.1712000370025635ms, incremental: 2 - 0.4526998996734619ms)
query\select-varbinary.js size=1048576 n=100: 79.54118101472912 (minor: 12 - 9.195600032806396ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-varbinary.js size=10485760 n=100: 9.140729389986111 (minor: 96 - 72.92129945755005ms, major: 81 - 331.9704990386963ms, incremental: 162 - 40.19900035858154ms

With the change:

node benchmarks\query\select-varbinary.js

query\select-varbinary.js size=10 n=10: 276.0097817866665 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-varbinary.js size=100 n=10: 172.34723141407457 (minor: 1 - 3.0879998207092285ms, major: 0 - 0ms, incremental: 2 - 9.315500020980835ms)
query\select-varbinary.js size=1000 n=10: 232.66581510978335 (minor: 1 - 2.0436999797821045ms, major: 0 - 0ms, incremental: 1 - 0.5453000068664551ms)
query\select-varbinary.js size=10000 n=10: 305.16367453683785 (minor: 1 - 1.7862999439239502ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-varbinary.js size=1048576 n=10: 39.571143772253826 (minor: 1 - 0.8203999996185303ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-varbinary.js size=10485760 n=10: 5.227353549286808 (minor: 10 - 13.137400388717651ms, major: 9 - 48.268799781799316ms, incremental: 18 - 5.6498987674713135ms)
query\select-varbinary.js size=10 n=100: 723.4455686788582 (minor: 1 - 1.609100103378296ms, major: 1 - 3.1033999919891357ms, incremental: 2 - 0.48549985885620117ms)
query\select-varbinary.js size=100 n=100: 754.2746630843649 (minor: 1 - 2.1960999965667725ms, major: 1 - 3.755999803543091ms, incremental: 2 - 0.43720006942749023ms)
query\select-varbinary.js size=1000 n=100: 600.5006974815601 (minor: 1 - 1.8889999389648438ms, major: 1 - 3.4354000091552734ms, incremental: 2 - 0.5083999633789062ms)
query\select-varbinary.js size=10000 n=100: 523.965665577866 (minor: 1 - 1.6253998279571533ms, major: 1 - 3.962399959564209ms, incremental: 2 - 0.5021998882293701ms)
query\select-varbinary.js size=1048576 n=100: 73.84582483460197 (minor: 12 - 8.558600425720215ms, major: 0 - 0ms, incremental: 0 - 0ms)
query\select-varbinary.js size=10485760 n=100: 8.222461727914647 (minor: 99 - 93.33590006828308ms, major: 91 - 379.2195978164673ms, incremental: 182 - 48.41420221328735ms)

Without the change:

node benchmarks\connection\open.js

connection\open.js n=10: 30.52480690007155 (minor: 1 - 1.5434000492095947ms, major: 1 - 2.651400089263916ms, incremental: 2 - 0.374500036239624ms)
connection\open.js n=100: 46.70442813089906 (minor: 4 - 5.734100103378296ms, major: 1 - 2.5404999256134033ms, incremental: 2 - 0.33460021018981934ms)

With the change:

node benchmarks\connection\open.js

connection\open.js n=10: 28.399183466676966 (minor: 1 - 1.6179001331329346ms, major: 1 - 3.0009000301361084ms, incremental: 2 - 0.4140000343322754ms)
connection\open.js n=100: 34.56079331674438 (minor: 4 - 6.981300115585327ms, major: 1 - 2.9261999130249023ms, incremental: 2 - 0.34850001335144043ms)

Without the change:

node benchmarks\bulk-load\iterable.js

bulk-load\iterable.js size=10 n=10: 155.19877082573504 (minor: 1 - 1.3322999477386475ms, major: 1 - 3.20770001411438ms, incremental: 2 - 0.33459997177124023ms)
bulk-load\iterable.js size=100 n=10: 108.51330264577135 (minor: 1 - 1.3799998760223389ms, major: 1 - 3.456700086593628ms, incremental: 2 - 0.3601999282836914ms)
bulk-load\iterable.js size=1000 n=10: 101.073501661143 (minor: 3 - 2.7444000244140625ms, major: 1 - 4.167399883270264ms, incremental: 2 - 0.40940022468566895ms)
bulk-load\iterable.js size=10000 n=10: 29.294397563643543 (minor: 16 - 18.773300170898438ms, major: 1 - 1.8295998573303223ms, incremental: 2 - 0.35809993743896484ms)
bulk-load\iterable.js size=10 n=100: 448.1997608406076 (minor: 2 - 2.1605000495910645ms, major: 1 - 3.2630999088287354ms, incremental: 2 - 0.3940999507904053ms)
bulk-load\iterable.js size=100 n=100: 422.3848779455538 (minor: 4 - 4.486100435256958ms, major: 1 - 2.3156001567840576ms, incremental: 2 - 0.2774999141693115ms)
bulk-load\iterable.js size=1000 n=100: 228.53272165788604 (minor: 17 - 18.84689950942993ms, major: 1 - 7.900500059127808ms, incremental: 2 - 0.6395995616912842ms)
bulk-load\iterable.js size=10000 n=100: 45.091800367236644 (minor: 106 - 75.29769897460938ms, major: 2 - 4.324300050735474ms, incremental: 4 - 0.6973998546600342ms)

With the change:

node benchmarks\bulk-load\iterable.js

bulk-load\iterable.js size=10 n=10: 226.61348803480783 (minor: 1 - 1.200700044631958ms, major: 1 - 2.914900064468384ms, incremental: 2 - 0.3115999698638916ms)
bulk-load\iterable.js size=100 n=10: 157.65410688948447 (minor: 1 - 1.2102999687194824ms, major: 1 - 3.2023000717163086ms, incremental: 2 - 0.42639994621276855ms)
bulk-load\iterable.js size=1000 n=10: 83.53137229514975 (minor: 3 - 3.2732999324798584ms, major: 1 - 5.573999881744385ms, incremental: 2 - 0.5929999351501465ms)
bulk-load\iterable.js size=10000 n=10: 27.15711695701246 (minor: 16 - 19.671200037002563ms, major: 1 - 1.79610013961792ms, incremental: 2 - 0.3268001079559326ms)
bulk-load\iterable.js size=10 n=100: 412.8060699004518 (minor: 2 - 2.458400011062622ms, major: 1 - 2.041900157928467ms, incremental: 2 - 0.2914001941680908ms)
bulk-load\iterable.js size=100 n=100: 269.2933903014524 (minor: 4 - 9.191299676895142ms, major: 1 - 5.903800010681152ms, incremental: 2 - 0.3736000061035156ms)
bulk-load\iterable.js size=1000 n=100: 146.13742705184984 (minor: 17 - 27.87279987335205ms, major: 1 - 11.896400213241577ms, incremental: 2 - 0.609299898147583ms)
bulk-load\iterable.js size=10000 n=100: 31.137066831634996 (minor: 106 - 98.0045006275177ms, major: 2 - 6.544100046157837ms, incremental: 4 - 1.397200107574463ms)

Without the change:

node benchmarks\request\rpcrequest-payload-varbinary.js

request\rpcrequest-payload-varbinary.js size=1048576 n=10: 1,619.3283026200731 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
request\rpcrequest-payload-varbinary.js size=10485760 n=10: 1,070.5377311023328 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
request\rpcrequest-payload-varbinary.js size=52428800 n=10: 1,055.4758084944694 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
request\rpcrequest-payload-varbinary.js size=1048576 n=100: 9,496.946731625783 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
request\rpcrequest-payload-varbinary.js size=10485760 n=100: 4,398.814079724106 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
request\rpcrequest-payload-varbinary.js size=52428800 n=100: 4,719.76401179941 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)

With the change:

node benchmarks\request\rpcrequest-payload-varbinary.js

request\rpcrequest-payload-varbinary.js size=1048576 n=10: 1,791.697274828445 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
request\rpcrequest-payload-varbinary.js size=10485760 n=10: 1,025.3465671396932 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
request\rpcrequest-payload-varbinary.js size=52428800 n=10: 617.8598570272292 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
request\rpcrequest-payload-varbinary.js size=1048576 n=100: 6,876.018510241829 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
request\rpcrequest-payload-varbinary.js size=10485760 n=100: 6,275.218534485463 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)
request\rpcrequest-payload-varbinary.js size=52428800 n=100: 6,364.9672204188155 (minor: 0 - 0ms, major: 0 - 0ms, incremental: 0 - 0ms)

@arthurschreiber
Copy link
Collaborator

arthurschreiber commented Oct 7, 2023 •
edited
Loading

@MichaelSun90 I assume the Node.js version you used was v16? The results are a bit all over the place, I'd have expected them to be a bit more consistent in the improvement. 🤔

Obviously, the token parsing ones should be the one that show the biggest difference, because that's the part that's most affected by the performance degradation coming from using class properties.

The benchmarks that test full request cycles are expectedly less affected, because token parsing is a small piece of the overall logic that gets executed in these benchmarks.

The bulk-load benchmarks and the request payload generation benchmark should also only show minimal changes, but there's quite some variation in there. If you have a lot of other processes running on your machines, that might make the benchmarks less accurate.

@arthurschreiber arthurschreiber changed the title feat: don't emit class properties for class definitions feat: improve performance on Node.js 16 by disabling class property definitions Oct 7, 2023
@arthurschreiber arthurschreiber merged commit 443701f into master Oct 8, 2023
27 of 29 checks passed
@arthurschreiber arthurschreiber deleted the michael-AddDeclare branch October 8, 2023 11:38
@arthurschreiber arthurschreiber mentioned this pull request Oct 8, 2023
Closed
@MichaelSun90
Copy link
Contributor Author

@MichaelSun90 I assume the Node.js version you used was v16? The results are a bit all over the place, I'd have expected them to be a bit more consistent in the improvement. 🤔

Obviously, the token parsing ones should be the one that show the biggest difference, because that's the part that's most affected by the performance degradation coming from using class properties.

The benchmarks that test full request cycles are expectedly less affected, because token parsing is a small piece of the overall logic that gets executed in these benchmarks.

The bulk-load benchmarks and the request payload generation benchmark should also only show minimal changes, but there's quite some variation in there. If you have a lot of other processes running on your machines, that might make the benchmarks less accurate.

I am on node 18.17.1. 🤔 The improvement on the token parsing part looks pretty promising.

@github-actions
Copy link

🎉 This PR is included in version 16.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@titobundy titobundy mentioned this pull request May 26, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arthurschreiber arthurschreiber arthurschreiber approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MichaelSun90 @arthurschreiber