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

Making type conversions explicit #1540

Merged
merged 18 commits into from May 24, 2019
Merged

Making type conversions explicit #1540

merged 18 commits into from May 24, 2019

Conversation

cos
Copy link
Contributor

@cos cos commented Jul 17, 2018 •
edited

Tests are still passing. Let me know if I can adjust anything.

The commits are fine-grained because they are generated automatically. Feel free to squash them on merge.

@benjamingr
Copy link
Collaborator

Things were explicitly done this way for static analysis. The engine was slightly slower with Number since one could define a Number function in a more local scope. This might not be the case anymore.

These changes would require benchmark runs to land.

@cos
Copy link
Contributor Author

cos commented Jul 20, 2018

@benjamingr, thanks for looking into this!

doxbee is now marginally faster with explicit type conversions, and there's no difference for the made-up parallel benchmark.

benchmarking master:

$ node tools/build --debug --release --zalgo --browser --minify

[... warmup]

$ time ./bench doxbee

file                                       time(ms)  memory(MB)
callbacks-baseline.js                           174       27.25
promises-bluebird-generator.js                  228       38.14
callbacks-suguru03-neo-async-waterfall.js       233       35.11
promises-bluebird.js                            275       47.87
promises-cujojs-when.js                         331       66.48
promises-lvivski-davy.js                        384       94.55
promises-tildeio-rsvp.js                        405       89.81
promises-native-async-await.js                  424      100.27
promises-ecmascript6-native.js                  461       99.10
generators-tj-co.js                             479       77.81
promises-then-promise.js                        484       71.37
callbacks-caolan-async-waterfall.js             648       79.89
promises-dfilatov-vow.js                        706      137.91
promises-obvious-kew.js                         830      124.27
promises-calvinmetcalf-lie.js                   869      157.75
streamline-generators.js                        880       80.03
promises-medikoo-deferred.js                   1035      107.13
observables-pozadi-kefir.js                    1208      141.82
streamline-callbacks.js                        1516      104.86
observables-Reactive-Extensions-RxJS.js        2423      183.55
promises-kriskowal-q.js                        8745      317.68
observables-caolan-highland.js                11730      541.29
observables-baconjs-bacon.js.js               26900      732.65

Platform info:
Darwin 17.7.0 x64
Node.JS 9.3.0
V8 6.2.414.46-node.15
Intel(R) Core(TM) i7-7820HQ CPU @ 2.90GHz × 8


real	1m8.040s
user	1m23.468s
sys	0m4.177s

[... another run]

real	1m8.700s
user	1m24.299s
sys	0m4.128s

[... warmup parallel]

$ time ./bench parallel

file                                      time(ms)  memory(MB)
callbacks-baseline.js                          375       76.91
promises-lvivski-davy.js                       528      162.41
promises-bluebird.js                           530       99.34
promises-bluebird-generator.js                 551      104.50
promises-cujojs-when.js                        643      171.76
callbacks-suguru03-neo-async-parallel.js       744       85.03
callbacks-caolan-async-parallel.js             830      157.45
promises-tildeio-rsvp.js                      1172      354.13
promises-ecmascript6-native.js                1338      304.28
promises-native-async-await.js                1526      320.95
promises-medikoo-deferred.js                  1866      280.45
promises-then-promise.js                      1985      312.96
promises-calvinmetcalf-lie.js                 2313      352.71
promises-dfilatov-vow.js                      2413      540.53
promises-obvious-kew.js                       2880      287.55
streamline-generators.js                     14480      975.98
streamline-callbacks.js                      38954     1180.19

[...]

real	1m19.523s
user	1m33.325s
sys	0m5.366s

[... another run of parallel]

real	1m19.208s
user	1m32.665s
sys	0m5.613s

benchmarking kale-code:explicit-type-conversions:

$ node tools/build --debug --release --zalgo --browser --minify

[... warmup]

$ time ./bench doxbee

file                                       time(ms)  memory(MB)
callbacks-baseline.js                           181       27.27
callbacks-suguru03-neo-async-waterfall.js       217       35.41
promises-bluebird-generator.js                  246       38.32
promises-bluebird.js                            284       47.94
promises-cujojs-when.js                         324       65.25
promises-lvivski-davy.js                        390       94.36
promises-tildeio-rsvp.js                        401       90.59
promises-native-async-await.js                  417      100.95
promises-ecmascript6-native.js                  444       96.20
generators-tj-co.js                             467       83.36
promises-then-promise.js                        497       67.25
callbacks-caolan-async-waterfall.js             641       80.92
promises-dfilatov-vow.js                        722      136.70
promises-obvious-kew.js                         794      102.67
streamline-generators.js                        878       82.68
promises-calvinmetcalf-lie.js                   897      157.99
observables-pozadi-kefir.js                    1129      142.01
promises-medikoo-deferred.js                   1167      108.16
streamline-callbacks.js                        1440      104.93
observables-Reactive-Extensions-RxJS.js        2412      183.18
observables-caolan-highland.js                 8719      517.09
promises-kriskowal-q.js                        8830      320.64
observables-baconjs-bacon.js.js               23768      732.59

[...]


real	1m2.030s
user	1m16.918s
sys	0m4.026s

[... another run]

real	0m59.075s
user	1m12.899s
sys	0m3.792s

[... warmup parallel]

$ time ./bench parallel

file                                      time(ms)  memory(MB)
callbacks-baseline.js                          406       77.09
callbacks-suguru03-neo-async-parallel.js       454       83.90
promises-lvivski-davy.js                       536      164.21
promises-bluebird.js                           537       98.54
promises-bluebird-generator.js                 567      105.21
promises-cujojs-when.js                        720      134.51
callbacks-caolan-async-parallel.js             824      155.20
promises-tildeio-rsvp.js                      1173      353.57
promises-ecmascript6-native.js                1348      303.50
promises-native-async-await.js                1454      318.50
promises-medikoo-deferred.js                  1837      299.55
promises-then-promise.js                      1918      314.24
promises-calvinmetcalf-lie.js                 2201      358.07
promises-obvious-kew.js                       2985      600.31
promises-dfilatov-vow.js                      2990      543.41
streamline-generators.js                     13857      909.18
streamline-callbacks.js                      38113     1125.26

[...]

real	1m18.390s
user	1m31.944s
sys	0m5.432s

[... another run of parallel]

real	1m19.463s
user	1m33.222s
sys	0m5.302s

@cos
Copy link
Contributor Author

cos commented Sep 4, 2018

hey @benjamingr, can you have a quick look at my message above?

@benjamingr
Copy link
Collaborator

@petkaantonov

kale-io and others added 13 commits March 7, 2019 14:57
@kale-io @cos
Explicit conversion between types
30f9a06 
Use explicit conversions between types (Number, String).
@kale-io @cos
Explicit conversion between types
1b37436 
Use explicit conversions between types (Number, String).
@kale-io @cos
Explicit conversion between types
486ca2b 
Use explicit conversions between types (Number, String).
@kale-io @cos
Explicit conversion between types
a6c2cdd 
Use explicit conversions between types (Number, String).
@kale-io @cos
Explicit conversion between types
c59c305 
Use explicit conversions between types (Number, String).
@kale-io @cos
Explicit conversion between types
77aa6cb 
Use explicit conversions between types (Number, String).
@kale-io @cos
Explicit conversion between types
606ccba 
Use explicit conversions between types (Number, String).
@kale-io @cos
Explicit conversion between types
23f66ce 
Use explicit conversions between types (Number, String).
@kale-io @cos
Explicit conversion between types
b9c6752 
Use explicit conversions between types (Number, String).
@kale-io @cos
Explicit conversion between types
e848889 
Use explicit conversions between types (Number, String).
@kale-io @cos
Explicit conversion between types
451f419 
Use explicit conversions between types (Number, String).
@kale-io @cos
Explicit conversion between types
c443d49 
Use explicit conversions between types (Number, String).
@cos
formatting
d794fd0
@cos cos force-pushed the explicit-type-conversions branch from 118fb16 to d794fd0 Compare March 7, 2019 22:57
@kale-io @cos
Explicit conversion between types
85288b2 
Use explicit conversions between types (Number, String).
@cos cos force-pushed the explicit-type-conversions branch from e206246 to 85288b2 Compare March 7, 2019 23:22
@kale-io @cos
Convert checks for missing indexOf to use includes
c4de987 
ECMAScript 7 introduced the includes function for arrays so that bitwise and comparisons to -1 are no longer needed
@kale-io @cos
Convert checks for missing indexOf to use includes
c472a31 
ECMAScript 7 introduced the includes function for arrays so that bitwise and comparisons to -1 are no longer needed
@kale-io @cos
Convert checks for missing indexOf to use includes
11e8705 
ECMAScript 7 introduced the includes function for arrays so that bitwise and comparisons to -1 are no longer needed
@kale-io @cos
Convert checks for missing indexOf to use includes
857b57c 
ECMAScript 7 introduced the includes function for arrays so that bitwise and comparisons to -1 are no longer needed
@cos
Copy link
Contributor Author

cos commented Mar 7, 2019

Hey @benjamingr and @petkaantonov, let me know if this is still of interest. I updated to the latest master and the tests are still passing.

Btw, the documentation is not in sync with the latest code: #1586

@benjamingr
Copy link
Collaborator

@petkaantonov ?

@petkaantonov petkaantonov merged commit 652dff3 into petkaantonov:master May 24, 2019
petkaantonov added a commit that referenced this pull request May 24, 2019
@petkaantonov
Revert "Making type conversions explicit (#1540)"
904f890 
This reverts commit 652dff3.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@cos @benjamingr @petkaantonov @kale-io