bucklescript-tea icon indicating copy to clipboard operation
bucklescript-tea copied to clipboard

Port Array and List to Belt

Open tcoopman opened this issue 6 years ago • 22 comments

I took a stab at #73, to see what's changed.

The test examples still work but I didn't check anything else yet. So please take a good look at this.

I think it would be valuable to test if the claims in #73 are true: smaller build size and better performance.

Build size

For the build size, I guess these should only be compared after using a bundler? Like rollup? Or is it fair to just compare the output of npm run build:test?

I already checked npm run build:test without belt:

ls -la lib/js/test/
total 372
drwxr-xr-x 2 thomas thomas   4096 Mar 20 09:19 ./
drwxr-xr-x 4 thomas thomas   4096 Mar 20 08:05 ../
-rw-r--r-- 1 thomas thomas 340985 Mar 20 09:19 app_test_client.js
-rw-r--r-- 1 thomas thomas   3677 Mar 20 09:19 test_client_attribute_removal.js
-rw-r--r-- 1 thomas thomas   1965 Mar 20 09:19 test_client_btn_update_span.js
-rw-r--r-- 1 thomas thomas   2504 Mar 20 09:19 test_client_counter.js
-rw-r--r-- 1 thomas thomas   7009 Mar 20 09:19 test_client_drag.js
-rw-r--r-- 1 thomas thomas    737 Mar 20 09:19 test_client.js
-rw-r--r-- 1 thomas thomas    131 Mar 20 08:05 tester.js

and with belt

ls -la lib/js/test/
total 400
drwxr-xr-x 2 thomas thomas   4096 Mar 20 09:26 ./
drwxr-xr-x 4 thomas thomas   4096 Mar 20 08:05 ../
-rw-r--r-- 1 thomas thomas 370563 Mar 20 09:26 app_test_client.js
-rw-r--r-- 1 thomas thomas   3693 Mar 20 09:26 test_client_attribute_removal.js
-rw-r--r-- 1 thomas thomas   1965 Mar 20 09:26 test_client_btn_update_span.js
-rw-r--r-- 1 thomas thomas   2504 Mar 20 09:26 test_client_counter.js
-rw-r--r-- 1 thomas thomas   7009 Mar 20 09:26 test_client_drag.js
-rw-r--r-- 1 thomas thomas    737 Mar 20 09:26 test_client.js
-rw-r--r-- 1 thomas thomas    131 Mar 20 08:05 tester.js

So before bundling the old release is smaller. So I tried to bundle with rollup and minifying:

ls -l release/
total 236
-rw-r--r-- 1 thomas thomas 126351 Mar 20 09:45 main.belt.js
-rw-r--r-- 1 thomas thomas 111677 Mar 20 09:46 main.js

as you can see the main.js (master) is still smaller main.belt.js (with belt).

This is the rollup config I used:

import node_resolve from 'rollup-plugin-node-resolve';
import uglify from 'rollup-plugin-uglify';

export default {
    input: './lib/js/test/app_test_client.js',
    output: {
        file: './release/main.js',
        format: 'iife'
    },
    plugins: [
        node_resolve({module: true, browser: true}),
        uglify()
    ],
    name: 'starter',
}

@bobzhang do you have an idea why belt is still larger? I can provide both app_test_client.js files if wanted, but it looks like all Belt functions are in there (shouldn't rollup remove these?)

Performance

Not sure how to test this? @OvermindDL1 do you have a benchmark that can be run?

End remarks

At the moment, the build size is not smaller and performance isn't measured yet, so I wouldn't merge this yet.

tcoopman avatar Mar 20 '18 08:03 tcoopman

a couple of things to check:

  1. not accidentally pull in List.js/Array.js and Belt_List.js/Belt_Array.js at the same time
  2. es6 mode
  3. Replace ocaml-stdlib Map with Belt.Map or Belt.Map.String will save a lot

I am always interested in performance/size, if @OvermindDL1 is interested in pushing in this direction, I am happy to invest some time in it

bobzhang avatar Mar 20 '18 09:03 bobzhang

Well, es6 mode helps a lot with rollup :open_mouth:

ls -l release/
total 68
-rw-r--r-- 1 thomas thomas 32843 Mar 20 11:57 main.belt.js
-rw-r--r-- 1 thomas thomas 32313 Mar 20 12:00 main.js

I got it down to 32733 after removed some OCaml_array imports, but still not as small as the old files. I'm going to update my PR later to remove the OCaml_array imports, but I have to revert some es6 things first.

tcoopman avatar Mar 20 '18 11:03 tcoopman

Not sure how to test this? @OvermindDL1 do you have a benchmark that can be run?

I do on my web server, shouldn't be hard to plug this in to test it later. :-)

OvermindDL1 avatar Mar 20 '18 14:03 OvermindDL1

I have a benchmark implementation here: https://github.com/utkarshkukreti/js-framework-benchmark/tree/bucklescript-tea. I'll be happy to test and report the results once the feedback by @OvermindDL1 is addressed. I haven't submitted it to the official repository because the implementation of key in this project is different than what they expect. I'm going to submit it as a non-keyed entry soon after @OvermindDL1 is happy with the implementation. :)

utkarshkukreti avatar Mar 20 '18 14:03 utkarshkukreti

I'm getting '|.' is not a valid value identifier for note when I'm trying to inspect it's type.

OvermindDL1 avatar Mar 20 '18 14:03 OvermindDL1

@utkarshkukreti nice, I was looking at doing the same thing with this benchmark, but now I can start from yours. I'm going to address the field issues

Edit: field was key

tcoopman avatar Mar 20 '18 15:03 tcoopman

Fixed the field function.

tcoopman avatar Mar 20 '18 16:03 tcoopman

@tcoopman feel free to start from that and submit it if you manage to address the keying feature. Just ping me when you do submit it!

utkarshkukreti avatar Mar 20 '18 16:03 utkarshkukreti

Just a FYI I ran your benchmark (still without the key issue) and these are the results:

tea with belt

{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"01_run1k","type":"cpu","min":152.109,"max":195.522,"mean":165.91140000000001,"median":162.8755,"geometricMean":165.47228311881423,"standardDeviation":12.378177201833875,"values":[152.109,153.738,155.256,160.772,162.495,163.256,167.193,171.863,176.91,195.522]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"02_replace1k","type":"cpu","min":41.266,"max":51.37,"mean":44.50430000000001,"median":43.1995,"geometricMean":44.380252541301914,"standardDeviation":3.406436555992199,"values":[41.266,41.615,41.903,42.436,43.016,43.383,43.668,46.239,50.147,51.37]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"03_update10th1k","type":"cpu","min":161.827,"max":212.437,"mean":182.25500000000005,"median":178.95499999999998,"geometricMean":181.67626146054565,"standardDeviation":14.707995689420096,"values":[161.827,164.265,175.769,176.562,177.262,180.648,180.743,196.216,196.821,212.437]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"04_select1k","type":"cpu","min":10.62,"max":14.027,"mean":11.8367,"median":11.415,"geometricMean":11.776622624903473,"standardDeviation":1.2141875514104068,"values":[10.62,10.651,10.667,10.9,11.252,11.578,12.056,13.192,13.424,14.027]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"05_swap1k","type":"cpu","min":11.441,"max":20.882,"mean":15.4519,"median":14.1955,"geometricMean":15.146409607584179,"standardDeviation":3.1620645929518902,"values":[11.441,12.392,13.444,13.456,14.106,14.285,15.385,19.425,19.703,20.882]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"06_remove-one-1k","type":"cpu","min":46.717,"max":60.73,"mean":54.17569999999999,"median":55.135000000000005,"geometricMean":54.04865795620652,"standardDeviation":3.6682672489882737,"values":[46.717,50.577,50.997,54.347,54.774,55.496,55.559,56.191,56.369,60.73]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"07_create10k","type":"cpu","min":1605.446,"max":1741.684,"mean":1678.9782,"median":1686.951,"geometricMean":1678.3190488659457,"standardDeviation":46.87854270943158,"values":[1605.446,1613.743,1624.73,1668.91,1682.331,1691.571,1718.475,1721.083,1721.809,1741.684]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"08_create1k-after10k","type":"cpu","min":397.1,"max":556.604,"mean":476.69899999999996,"median":463.488,"geometricMean":472.3280873460287,"standardDeviation":64.66153213619363,"values":[397.1,412.467,415.356,416.345,431.084,495.892,538.702,549.571,553.869,556.604]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"09_clear10k","type":"cpu","min":184.377,"max":207.629,"mean":194.24429999999998,"median":194.9745,"geometricMean":194.10130126281874,"standardDeviation":7.474683592634535,"values":[184.377,186.091,186.973,187.563,194.363,195.586,196.761,200.263,202.837,207.629]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"21_ready-memory","type":"memory","min":2.5645294189453125,"max":2.8879547119140625,"mean":2.8508331298828127,"median":2.8799781799316406,"geometricMean":2.849131039260525,"standardDeviation":0.0955033788232127,"values":[2.5645294189453125,2.8787155151367188,2.8788986206054688,2.8790512084960938,2.8797760009765625,2.8801803588867188,2.8848190307617188,2.8868789672851562,2.8875274658203125,2.8879547119140625]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"22_run-memory","type":"memory","min":8.850395202636719,"max":9.144691467285156,"mean":8.903846740722656,"median":8.852703094482422,"geometricMean":8.903360250417679,"standardDeviation":0.09363650167086045,"values":[8.850395202636719,8.850685119628906,8.850845336914062,8.851058959960938,8.852676391601562,8.852729797363281,8.861305236816406,8.914192199707031,9.0098876953125,9.144691467285156]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"23_update5-memory","type":"memory","min":9.094764709472656,"max":9.39617919921875,"mean":9.294584655761719,"median":9.387943267822266,"geometricMean":9.293808184562282,"standardDeviation":0.11986293559733954,"values":[9.094764709472656,9.15167236328125,9.153778076171875,9.203201293945312,9.387916564941406,9.387969970703125,9.388137817382812,9.390396118164062,9.391830444335938,9.39617919921875]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"24_run5-memory","type":"memory","min":8.940299987792969,"max":9.270462036132812,"mean":9.010234832763672,"median":8.948631286621094,"geometricMean":9.009361494532309,"standardDeviation":0.1263220433836874,"values":[8.940299987792969,8.942001342773438,8.945487976074219,8.946182250976562,8.948135375976562,8.949127197265625,8.949142456054688,8.956687927246094,9.25482177734375,9.270462036132812]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"25_run-clear-memory","type":"memory","min":3.0928802490234375,"max":3.1078872680664062,"mean":3.100122833251953,"median":3.0995864868164062,"geometricMean":3.100119531503306,"standardDeviation":0.004525022196291292,"values":[3.0928802490234375,3.0951690673828125,3.09735107421875,3.0974273681640625,3.0992202758789062,3.0999526977539062,3.1012954711914062,3.1039505004882812,3.1060943603515625,3.1078872680664062]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"30_startup-ci","type":"memory","min":69.876,"max":83.052,"mean":76.4164,"median":75.9015,"geometricMean":76.28539302101578,"standardDeviation":4.467489881353958,"values":[69.876,70.008,73.299,74.264,75.542,76.261,79.258,80.687,81.917,83.052]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"30_startup-bt","type":"memory","min":16.05399990081787,"max":28.841000020503998,"mean":20.02409999370575,"median":19.066499948501587,"geometricMean":19.62645045286612,"standardDeviation":4.237983538832022,"values":[16.05399990081787,16.34500002861023,16.488000094890594,16.967000007629395,18.437999963760376,19.694999933242798,20.14400005340576,20.316999971866608,26.951999962329865,28.841000020503998]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"30_startup-mainthreadcost","type":"memory","min":157.56600004434586,"max":211.88899993896484,"mean":178.04220004677774,"median":177.89450019598007,"geometricMean":177.4910708711649,"standardDeviation":14.2929872999324,"values":[157.56600004434586,163.83100014925003,166.06399977207184,174.9579999446869,177.32200014591217,178.46700024604797,179.10300034284592,183.78500002622604,187.4369998574257,211.88899993896484]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"30_startup-totalbytes","type":"memory","min":192008,"max":192008,"mean":192008,"median":192008,"geometricMean":192008.00000000012,"standardDeviation":0,"values":[192008,192008,192008,192008,192008,192008,192008,192008,192008,192008]},

tea without belt

{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"01_run1k","type":"cpu","min":146.25,"max":194.257,"mean":163.50660000000002,"median":158.38600000000002,"geometricMean":162.84581320751255,"standardDeviation":15.034914540495404,"values":[146.25,147.902,151.668,156.381,157.776,158.996,167.816,168.459,185.561,194.257]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"02_replace1k","type":"cpu","min":43.225,"max":149.978,"mean":56.251200000000004,"median":44.417,"geometricMean":51.518479782247105,"standardDeviation":31.360674810341695,"values":[43.225,43.629,43.909,44.168,44.348,44.486,47.92,48.773,52.076,149.978]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"03_update10th1k","type":"cpu","min":164.462,"max":206.829,"mean":185.88129999999998,"median":188.58499999999998,"geometricMean":185.41493944421717,"standardDeviation":13.114208592591476,"values":[164.462,170.753,173.096,178.002,188.333,188.837,191.957,196.139,200.405,206.829]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"04_select1k","type":"cpu","min":11.791,"max":15.401,"mean":13.5716,"median":13.5925,"geometricMean":13.519825682444605,"standardDeviation":1.1833931045937358,"values":[11.791,12.344,12.399,12.696,13.145,14.04,14.515,14.562,14.823,15.401]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"05_swap1k","type":"cpu","min":12.765,"max":19.314,"mean":15.677999999999997,"median":15.6985,"geometricMean":15.518668433859979,"standardDeviation":2.2526869289805895,"values":[12.765,12.918,13.408,14.464,15.243,16.154,16.559,16.747,19.208,19.314]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"06_remove-one-1k","type":"cpu","min":47.06,"max":57.943,"mean":52.133500000000005,"median":51.489999999999995,"geometricMean":52.03602850654032,"standardDeviation":3.200191627074853,"values":[47.06,49.629,49.674,49.886,50.323,52.657,53.599,55.175,55.389,57.943]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"07_create10k","type":"cpu","min":1615.334,"max":2421.554,"mean":1781.2675,"median":1709.3204999999998,"geometricMean":1768.8113121116621,"standardDeviation":227.68166555884557,"values":[1615.334,1621.969,1647.524,1683.374,1685.445,1733.196,1745.62,1756.327,1902.332,2421.554]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"08_create1k-after10k","type":"cpu","min":392.692,"max":550.713,"mean":449.0959,"median":446.2665,"geometricMean":447.0431070749802,"standardDeviation":44.03106179153529,"values":[392.692,404.047,410.613,431.921,445.927,446.606,452.644,463.994,491.802,550.713]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"09_clear10k","type":"cpu","min":185.141,"max":197.791,"mean":190.7775,"median":188.958,"geometricMean":190.7137864369707,"standardDeviation":4.94101498176235,"values":[185.141,186.151,186.213,186.288,187.043,190.873,195.423,195.579,197.273,197.791]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"21_ready-memory","type":"memory","min":2.57476806640625,"max":2.8864212036132812,"mean":2.8491287231445312,"median":2.8788681030273438,"geometricMean":2.8475698943835646,"standardDeviation":0.09148787807961953,"values":[2.57476806640625,2.8768081665039062,2.8777923583984375,2.877960205078125,2.8787002563476562,2.8790359497070312,2.8791122436523438,2.8801956176757812,2.8804931640625,2.8864212036132812]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"22_run-memory","type":"memory","min":8.801239013671875,"max":9.155044555664062,"mean":8.883805847167968,"median":8.853404998779297,"geometricMean":8.883327720563287,"standardDeviation":0.09293082525826366,"values":[8.801239013671875,8.847610473632812,8.849891662597656,8.851997375488281,8.853378295898438,8.853431701660156,8.861343383789062,8.873619079589844,8.8905029296875,9.155044555664062]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"23_update5-memory","type":"memory","min":9.103347778320312,"max":9.399925231933594,"mean":9.243565368652344,"median":9.16195297241211,"geometricMean":9.242793920760922,"standardDeviation":0.11960343377757948,"values":[9.103347778320312,9.147636413574219,9.152969360351562,9.153060913085938,9.1585693359375,9.165336608886719,9.381187438964844,9.383872985839844,9.389747619628906,9.399925231933594]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"24_run5-memory","type":"memory","min":8.943099975585938,"max":8.962791442871094,"mean":8.9528564453125,"median":8.95302963256836,"geometricMean":8.952853923233794,"standardDeviation":0.0067200587073721375,"values":[8.943099975585938,8.944679260253906,8.945106506347656,8.9501953125,8.952804565429688,8.953254699707031,8.956207275390625,8.958786010742188,8.961639404296875,8.962791442871094]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"25_run-clear-memory","type":"memory","min":3.069732666015625,"max":3.10784912109375,"mean":3.097010040283203,"median":3.0989341735839844,"geometricMean":3.0969946583940313,"standardDeviation":0.0097403180817445,"values":[3.069732666015625,3.0950927734375,3.0964584350585938,3.0988311767578125,3.0989227294921875,3.0989456176757812,3.09979248046875,3.0999908447265625,3.1044845581054688,3.10784912109375]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"30_startup-ci","type":"memory","min":62.733,"max":74.892,"mean":68.8932,"median":68.3835,"geometricMean":68.7797091877475,"standardDeviation":3.9528779085623182,"values":[62.733,63.937,65.901,66.992,68.229,68.538,71.248,72.382,74.08,74.892]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"30_startup-bt","type":"memory","min":13.940999984741211,"max":20.912999987602234,"mean":18.300599998235704,"median":18.15450006723404,"geometricMean":18.17640601340692,"standardDeviation":2.0644723971419157,"values":[13.940999984741211,15.88099992275238,17.817999958992004,17.83399999141693,17.874000132083893,18.435000002384186,19.76800000667572,19.84299999475479,20.699000000953674,20.912999987602234]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"30_startup-mainthreadcost","type":"memory","min":158.4250002503395,"max":192.99400025606155,"mean":171.8747000873089,"median":169.72149994969368,"geometricMean":171.5253346640075,"standardDeviation":11.04497936960999,"values":[158.4250002503395,159.7850000858307,160.36300003528595,164.9220004081726,169.0969998240471,170.34600007534027,178.42500030994415,182.185999751091,182.203999876976,192.99400025606155]},
{"framework":"bucklescript-tea-v0.7.2-keyed","benchmark":"30_startup-totalbytes","type":"memory","min":192008,"max":192008,"mean":192008,"median":192008,"geometricMean":192008.00000000012,"standardDeviation":0,"values":[192008,192008,192008,192008,192008,192008,192008,192008,192008,192008]},];

as you can see, some a bit slower, some a bit faster, but largely in the same ballpark.

tcoopman avatar Mar 20 '18 19:03 tcoopman

as you can see, some a bit slower, some a bit faster, but largely in the same ballpark.

Hmm, and I have algorithmic changes in mind that could speed it up quite well regardless, and if these changes really have such little effect then so far it doesn't seem worth it. Anyone have any further ideas on how to increase performance across the board via code call changes and not algorithmic changes (since those are about to change anyway) so I can absorb the ideas? I don't want to close this unless this idea has been pretty well looked through because it seems like it could be a good idea excepting some odd naming and weird |. that I really don't like. :-)

OvermindDL1 avatar Mar 20 '18 20:03 OvermindDL1

Keep in mind our version is stack safe. A conclusion about benchmark shouldn’t be drawn so soon

bobzhang avatar Mar 21 '18 00:03 bobzhang

@bobzhang Is there any blog post to tell us why you make Belt and want us to replace ocaml std lib with it? I mean some insights on why Belt is more superior. Thank you.

I only found this

jackalcooper avatar Mar 21 '18 03:03 jackalcooper

Keep in mind our version is stack safe. A conclusion about benchmark shouldn’t be drawn so soon

I'd like to point out that I'm using Belt functions in the benchmark implementation already because OCaml's default implementations were causing stack overflow errors when e.g. concatenating a list of 10k items.

utkarshkukreti avatar Mar 21 '18 07:03 utkarshkukreti

I'd like to point out that I'm using Belt functions in the benchmark implementation already because OCaml's default implementations were causing stack overflow errors when e.g. concatenating a list of 10k items.

Yeah I saw that. I would say that that's a good reason to use Belt for TEA as well?

tcoopman avatar Mar 21 '18 07:03 tcoopman

@tcoopman can you try to replace Map.Make(String) with Belt.Map.String, you should see some obvious boost? Also the size comparison should be the output of closure or uglifyjs, since the rollup output still contains some comments

bobzhang avatar Mar 21 '18 08:03 bobzhang

I replaced Map.Make(String) and got following improvements

new size 29624 from 32313 without belt. This looks like a nice improvement! (The output of these numbers is this pipeline: rollup > uglify)

I'll try to run the benchmark later again. But we have at this point at least 2 positives for Belt:

  • smaller size: correct!
  • no stack overflow!

tcoopman avatar Mar 21 '18 13:03 tcoopman

I'd like to point out that I'm using Belt functions in the benchmark implementation already because OCaml's default implementations were causing stack overflow errors when e.g. concatenating a list of 10k items.

OCaml uses TCO, if javascript is a crappy enough VM to not support TCO yet then the runtime needs to be adjusted to support it, such as using tests to support trampolining out and rebuilding (which would not be hit most of the time anyway and even when it does it's still not that bad considering how rare it is in the call path). That is not a fault of the OCaml standard library, it's a fault of the converted code that needs to use 'something' (the most trivial trampoline to implement would be returning a continuation function from the deep depth to be finally tested and if that trampoline function is returned then call it again and repeat, else pass the return value on to where it wants to be used) on a non-TCO VM.

Yeah I saw that. I would say that that's a good reason to use Belt for TEA as well?

Instead of making a new standard library, why not change the internal code of the actual standard library to use loops instead? That way the user code is the same.

I think that is what I am not understanding about why Belt is created, from what I see:

  1. Belt uses some really, like REALLY BAD and nonsensical names for some functions.
  2. The argument ordering is very wrong.
  3. It is not following OCaml convention of snake_case and instead using this weird camelCase (yes I know the play on words), I get enough of that camelCase by having to follow Elm's API... ;-)
  4. Everything it seems that it tries to do with not blowing the stack could be done by just fixing up the internal parts of the standard library without changing the public API as far as what it would fix up (wouldn't fix other libraries or user code) or better yet fix the code generator to return continuations when necessary (which would fix other libraries and user code).

So I guess, what are the reasons for the above 4 points? The 3'rd point is mostly visual so it's less of an issue, but the other 3, especially the 4th are big things...

OvermindDL1 avatar Mar 21 '18 14:03 OvermindDL1

Are u aware ocaml stdlib is not stack safe in native either, some of my benchmarks show that the js lib is almost as fast as native. Is it really cool to blame everything on js without any investigation...

bobzhang avatar Mar 21 '18 14:03 bobzhang

OCaml uses TCO

Does OCaml Native also optimize functions that it says are "not tail-recursive", e.g. List.append? Because calling that function was causing a stack overflow in my benchmark implementation.

utkarshkukreti avatar Mar 21 '18 14:03 utkarshkukreti

Are u aware ocaml stdlib is not stack safe in native either, some of my benchmarks show that the js lib is almost as fast as native. Is it really cool to blame everything on js without any investigation...

I was speaking of TCO, not non-TCO recursive.

Does OCaml Native also optimize functions that it says are "not tail-recursive", e.g. List.append? Because calling that function was causing a stack overflow in my benchmark implementation.

Some functions are indeed non-TCO Recursive, and they tend to be documented as such and if you have a large input in one of those functions then you should use a different style that is TCO Recursive or a loop. :-)

TCO functions should never blow a stack themselves though, ever.

OvermindDL1 avatar Mar 21 '18 14:03 OvermindDL1

@OvermindDL1 is this something you're interested into completing? If not I'll close it.

tcoopman avatar Apr 23 '18 11:04 tcoopman

@OvermindDL1 is this something you're interested into completing? If not I'll close it.

@tcoopman As it stands it doesn't have a substantial enough performance benefit to warrant inclusion especially as it lacks native compilation at that point. Once Belt can be natively compiled at on-par (within a decent range specifically) with the standard library and Belt supplies an on-average performance enhancement that is well above the built-in library in any use-case, then I'll definitely want to include it. For now though I'd like to leave it in limbo until those are complete. :-)

OvermindDL1 avatar Apr 23 '18 14:04 OvermindDL1