Port Array and List to Belt #74

tcoopman · 2018-03-20T08:52:29Z

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.

bobzhang · 2018-03-20T09:11:46Z

a couple of things to check:

not accidentally pull in List.js/Array.js and Belt_List.js/Belt_Array.js at the same time
es6 mode
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

tcoopman · 2018-03-20T11:32:36Z

Well, es6 mode helps a lot with rollup 😮

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.

OvermindDL1

In addition to a few other questions I have one big big question: This needs to be able to compile soonish by the stock native OCaml compiler to allow for server-sided html generation (I'm about to plug it into the server to act just as such here soon at work), thus is the Belt.* and |. and so forth usable by the native compiler? If it is not then it needs to be proper guarded and use the original OCaml functions via platform tests.

I'm always interested in performance improvements, but it needs to work with the native compiler so it supports server-sided template generation. ^.^

OvermindDL1 · 2018-03-20T14:27:28Z

src/tea_html.ml

@@ -224,8 +224,8 @@ let class' name = prop "className" name

 let classList classes =
  classes
-  |> List.filter (fun (_fst, snd) -> snd)
-  |> List.map (fun (fst, _snd) -> fst)
+  |. Belt.List.keep (fun (_fst, snd) -> snd)


What on earth is keep? I've never heard of a filter function being called keep in any language I've used in 30 years... o.O

Any chance of using a proper descriptive name?

Also, |. is... odd looking, what does it gain here?

I agree with keep -> filter. Belt functions have the data structure as the first argument which doesn't work well with a sequence of |>. |. was added as an alternative. a |. b c = b a c.

Except I can't seem to find where |. is defined. The data structure should be at the end, not the start, else currying because quite a bit more difficult (why on earth put it at the start in a currying language?!?).

I agree, but I'm going to cc @bobzhang. I've just implemented everything with Belt, so I'm not the biggest fan of the names either.

Except I can't seem to find where |. is defined.

I think it's part of the syntax? See rescript-lang/rescript#2620.

The data structure should be at the end, not the start, else currying because quite a bit more difficult (why on earth put it at the start in a currying language?!?).

Yes, even with |. I think this might not be a great idea. Now some APIs will use |> and some will use |. resulting in (IMO) unnecessary complexity as can be seen in this diff itself.

Also, since it prepends it, then what happens if something like this is done:

let a b c d = b + c + d let aa = a 1 let aaa = 2 |. aa 3

Will aaa end up calling a 2 1 3 as I'd expect if it stuffs in front or would it end up calling a 1 2 3, which I'd might also expect considering that's more textual though not really correct for the definition?

Things like |> that just adds an argument to the current curry set is trivial...

Tested both of these:

let aaaa = let a b c d = b + c * d in 2 |. a 1 3 let aaaa' = let a b c d = b + c * d in 2 |. (a 1) 3

And they return different results... That is very wtf'y... I'm not liking this |. operator at all...
:-(

I'm not sure why you wouldn't expect different results here. Just from looking at it, I think would expect aaaa to be: a 2 1 3 and aaaa' to be a 1 2 3

To be clear, I'm not sure if I like the |. yet, but I'm willing to try it. Elixir has only currying like this. Furthermore, there were some arguments in the proposal (reasonml/reason#1452)

Elixir doesn't have partial application though. For languages without partial application then front-first piping makes sense (and I actually prefer it this way as it reads more naturally as well), but for languages with partial application (like OCaml) then piping should always only ever pipe into the current pending (end) position. So someone even proposing an Elixir-like piping operator into OCaml does not know the language and does not understand the ramifications (like the 2 |. a 1 3 and 2 |. (a 1) 3 differences is an obvious design fault, obvious 'why' it happens, but it is surprising nonetheless as it just should not happen in a language with partial application).

Actually, if the purpose of front-filling was just for JS devs side stuff, then why doesn't bucklescript 'flip' the arguments in all functions and calls when it compiles to javascript? That way the final 'data' argument in OCaml is the first argument in Javascript (thus also making named and optional arguments later in the Javascript argument list as well, which is indeed inverted from the OCaml standard style because OCaml has partial application).

OvermindDL1 · 2018-03-20T14:28:44Z

src/tea_json.ml

              with ParseFail e -> Tea_result.Error ("Invalid dict parsing: " ^ e)
            )
          | _ -> Tea_result.Error "Non-dict value"
      )

-  let field key (Decoder decoder) =
+  let field (Decoder decoder) key =


This breaks the Elm api here, must not be done for anything in the main 'Tea' namespace! Ditto for the rest in this module!

ok, going to change this

OvermindDL1 · 2018-03-20T14:29:16Z

src/tea_json.ml

@@ -168,15 +168,15 @@ module Decoder = struct
      )

  let at fields dec =
-    List.fold_right field fields dec
+    Belt.List.reduceReverse fields dec field


What on earth is reduceReverse? I've never heard that name in any language in 30 years either. Can a more descriptive/appropriate name be used?

Looks like it's going to be renamed to reduceRight: rescript-lang/rescript#2631.

Ah, so it doesn't reverse the output, it is just a right fold (reversed input), that makes more sense than it being called 'reduceReverse'. ^.^;

What is with all these very non-standard names in Belt I'm curious? o.O?

OvermindDL1 · 2018-03-20T14:31:12Z

src/vdom.ml

@@ -202,7 +202,7 @@ let patchVNodesOnElems_PropertiesApply_Add callbacks elem _idx = function
  | Attribute (namespace, k, v) -> Web.Node.setAttributeNsOptional elem namespace k v
  | Data (k, v) -> Js.log ("TODO:  Add Data Unhandled", k, v); failwith "TODO:  Add Data Unhandled"
  | Event (name, handlerType, cache) -> cache := eventHandler_Register callbacks elem name handlerType
-  | Style s -> List.fold_left (fun () (k, v) -> Web.Node.setStyleProperty elem k (Js.Null.return v)) () s
+  | Style s -> Belt.List.reduceU s () (fun [@bs] () (k, v) -> Web.Node.setStyleProperty elem k (Js.Null.return v))


what is reduceU? Never heard this function name and have no clue what it does with that odd U postfix (also, why is it not following OCaml standards of snake_case for standard library calls?). What does it do and why is it being used?

from the docs:

For higher order functions, it will be suffixed U if it takes uncurried callback. val forEach : 'a t -> ('a -> unit) -> unit val forEachU : 'a t -> ('a -> unit [@bs]) -> unit In general, uncurried version will be faster, but it may be less familiar to people who have a background in functional programming.

https://bucklescript.github.io/bucklescript/api/Belt.html

Shouldn't uncurrying be part of the optimizer and not something the coder has to deal with? o.O

OvermindDL1 · 2018-03-20T14:31:27Z

src/vdom.ml

@@ -232,7 +232,7 @@ let patchVNodesOnElems_PropertiesApply_Mutate _callbacks elem _idx oldProp = fun
    (* let () = Js.log ("Mutating Style", elem, oldProp, _newProp) in *)
    match [@ocaml.warning "-4"] oldProp with
    | Style oldS ->
-      List.fold_left2 (fun () (ok, ov) (nk, nv) ->
+      Belt.List.reduce2U oldS s () (fun [@bs] () (ok, ov) (nk, nv) ->


Again a non-descriptive U postfix?

OvermindDL1 · 2018-03-20T14:31:50Z

src/web_xmlhttprequest.ml

-      |> Array.to_list
-      |> List.filter (fun a -> Array.length a == 2)
-      |> List.map
+      |. Belt.Array.map (Js.String.splitAtMost ": " ~limit:2)


What is with all these |. and where is the definition of it defined?

|. should work in 2.2.3

How do you inspect it's type? I'm trying to type-hole it but it just says it doesn't exist...

OvermindDL1 · 2018-03-20T14:32:17Z

test/test_client_attribute_removal.ml

@@ -39,7 +39,7 @@ let render_languages selected languages =
    | Some l -> language == l
    | None -> false
  in
-  let rendered = List.map (fun l -> lang l (is_selected selected l)) languages in
+  let rendered = Belt.List.mapU languages (fun [@bs] l -> lang l (is_selected selected l)) in


A U postfix on a map function now? What does this mean?!

OvermindDL1 · 2018-03-20T14:36:28Z

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. :-)

utkarshkukreti · 2018-03-20T14:41:51Z

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. :)

OvermindDL1 · 2018-03-20T14:41:52Z

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

tcoopman · 2018-03-20T15:53:31Z

@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 · 2018-03-20T16:01:14Z

Fixed the field function.

utkarshkukreti · 2018-03-20T16:25:44Z

@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!

tcoopman · 2018-03-20T19:21:58Z

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.

OvermindDL1 · 2018-03-20T20:01:03Z

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. :-)

bobzhang · 2018-03-21T00:31:35Z

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

jackalcooper · 2018-03-21T03:10:14Z

@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

utkarshkukreti · 2018-03-21T07:23:28Z

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.

tcoopman · 2018-03-21T07:52:28Z

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?

bobzhang · 2018-03-21T08:51:55Z

@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

tcoopman · 2018-03-21T13:36:50Z

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!

OvermindDL1 · 2018-03-21T14:21:57Z

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:

Belt uses some really, like REALLY BAD and nonsensical names for some functions.
The argument ordering is very wrong.
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... ;-)
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...

bobzhang · 2018-03-21T14:48:39Z

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

utkarshkukreti · 2018-03-21T14:52:33Z

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.

OvermindDL1 · 2018-03-21T14:56:50Z

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.

tcoopman · 2018-04-23T11:13:00Z

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

OvermindDL1 · 2018-04-23T14:35:16Z

@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. :-)

tcoopman added 4 commits March 20, 2018 08:04

update packages

02dd136

recompiled with 2.2.3

057b4f1

replace Array with Belt.Array when possible

6ba3fcc

replace List with Belt.List

9a6c511

OvermindDL1 requested changes Mar 20, 2018

View reviewed changes

tcoopman added 2 commits March 20, 2018 16:54

use Belt.Array in stead of Ocaml array

9aad34f

reverse field change

fff9016

replace Map.Make by Belt map

8aca9b9

Port Array and List to Belt #74

Are you sure you want to change the base?

Port Array and List to Belt #74

Conversation

tcoopman commented Mar 20, 2018

Build size

Performance

End remarks

bobzhang commented Mar 20, 2018

tcoopman commented Mar 20, 2018

OvermindDL1 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OvermindDL1 Mar 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OvermindDL1 Mar 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OvermindDL1 commented Mar 20, 2018

utkarshkukreti commented Mar 20, 2018

OvermindDL1 commented Mar 20, 2018

tcoopman commented Mar 20, 2018 • edited Loading

tcoopman commented Mar 20, 2018

utkarshkukreti commented Mar 20, 2018

tcoopman commented Mar 20, 2018

OvermindDL1 commented Mar 20, 2018

bobzhang commented Mar 21, 2018

jackalcooper commented Mar 21, 2018

utkarshkukreti commented Mar 21, 2018

tcoopman commented Mar 21, 2018 • edited Loading

bobzhang commented Mar 21, 2018

tcoopman commented Mar 21, 2018

OvermindDL1 commented Mar 21, 2018 • edited Loading

bobzhang commented Mar 21, 2018

utkarshkukreti commented Mar 21, 2018

OvermindDL1 commented Mar 21, 2018

tcoopman commented Apr 23, 2018

OvermindDL1 commented Apr 23, 2018 • edited Loading

OvermindDL1 left a comment •

edited

Loading

OvermindDL1 Mar 20, 2018 •

edited

Loading

OvermindDL1 Mar 20, 2018 •

edited

Loading

tcoopman commented Mar 20, 2018 •

edited

Loading

tcoopman commented Mar 21, 2018 •

edited

Loading

OvermindDL1 commented Mar 21, 2018 •

edited

Loading

OvermindDL1 commented Apr 23, 2018 •

edited

Loading