Update dependencies and use esbuild for bundling #303

triallax · 2022-05-27T10:12:36Z

You may wonder why all these changes are done in one commit, and that's because they're all kind of interrelated:

I initially updated PureScript to 0.15.x, which produces code that uses ES modules instead of CommonJS ones
The version of purescript-decimals in the 0.15.x package set was tested with decimal.js 10.3.1, so use that
Since we're using ES modules now, I updated clipboardy and xdg-basedir to versions that use ES modules (I admit that I didn't have to do this one in this commit)
pulp browserify doesn't work with PureScript 0.15.x, so I had to migrate to something else (I chose esbuild)

Side note: purescript-parsing 9.0.0 brings with it a wonderful performance boost, which speeds up the test suite by 3 times on my machine. Before:

Command	Mean [s]	Min [s]	Max [s]	Relative
`npm test`	102.231 ± 0.485	101.749	103.464	1.00

After:

Command	Mean [s]	Min [s]	Max [s]	Relative
`npm test`	34.666 ± 0.299	34.173	34.985	1.00

Feel free to review this, but I'm still considering some things:

Maybe we can try out swc instead of esbuild
Should we migrate to Spago? It supports esbuild out of the box. This may be more suitable for another PR though.

sharkdp · 2022-05-27T12:13:59Z

Thank you!

Just on mobile right now, but concerning the JS bundler: I personally don't care too much about the speed of the bundling itself, but rather about the speed and size of the generated code. I don't know much about the process, but I guess there could be significant differences w.r.t. dead code elimination etc.

I have some ideas for a few Insect benchmarks . So we could also postpone that decision until we have a quantitative measure available.

Concerning the parsing speed improvement: Awesome!

sharkdp · 2022-05-27T12:14:34Z

Oh and concerning spago: I'll leave that up to you.

triallax · 2022-05-27T16:10:27Z

I read a bit about SWC, and I think it's not a good fit for Insect, as it seems to be a bit immature.

Just on mobile right now, but concerning the JS bundler: I personally don't care too much about the speed of the bundling itself, but rather about the speed and size of the generated code. I don't know much about the process, but I guess there could be significant differences w.r.t. dead code elimination etc.

Can't say much about anything other than esbuild, but it does seem to be doing minification fairly well: the insect.js produced by it is 351508 characters long, versus the 411477-character-long one hosted right now on https://insect.sh (to be fair, I have no idea what you used to minify that).

I have some ideas for a few Insect benchmarks . So we could also postpone that decision until we have a quantitative measure available.

Absolutely.

Oh and concerning spago: I'll leave that up to you.

Sure, I'll think about it.

I'll mark this PR as ready for review; feel free to take a closer look at the changes.

sharkdp · 2022-05-27T19:27:04Z

I read a bit about SWC, and I think it's not a good fit for Insect, as it seems to be a bit immature.

ok

Can't say much about anything other than esbuild, but it does seem to be doing minification fairly well: the insect.js produced by it is 351508 characters long, versus the 411477-character-long one hosted right now on https://insect.sh (to be fair, I have no idea what you used to minify that).

Oh, nice! I am currently using the closure-compiler to minify.

I have some ideas for a few Insect benchmarks . So we could also postpone that decision until we have a quantitative measure available.

Absolutely.

See #304 for a start. I benchmarked master vs this branch. As you already noted, parsing speed is significantly faster - very nice! Unfortunately, startup time is significantly slower (40% for simple one-off computations, 70% for starting interactive mode).

master

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`Startup time (insect '1+1')`	261.6 ± 2.9	257.5	267.0	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`Startup time, interactive mode (insect < computations-1.ins)`	357.2 ± 8.0	347.9	374.4	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`Evaluation speed (insect < computations-160.ins)`	2132.4 ± 34.5	2076.4	2188.4	1.00

this branch

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`Startup time (insect '1+1')`	445.0 ± 14.9	419.5	474.9	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`Startup time, interactive mode (insect < computations-1.ins)`	489.0 ± 9.5	476.2
510.5	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`Evaluation speed (insect < computations-160.ins)`	1358.4 ± 28.5	1319.6	1408.0	1.00

triallax · 2022-05-27T19:30:53Z

Unfortunately, startup time is significantly slower (40% for simple one-off computations, 70% for starting interactive mode).

My gut tells me it's because everything is imported at the top in index.js, even if unneeded. Maybe we can use dynamic imports to import modules only when needed?

triallax · 2022-05-27T19:38:48Z

Also, just to clear up potential misunderstandings: index.js does not at all depend on the bundled and minified JS file (nor did it before). So, what you're measuring does not depend on esbuild's output at all.

sharkdp · 2022-05-27T19:43:15Z

Also, just to clear up potential misunderstandings: index.js does not at all depend on the bundled and minified JS file (nor did it before). So, what you're measuring does not depend on esbuild's output at all.

No, right. But it does depend on the PureScript 0.15 changes and dependency upgrades (leading to faster parsing speed) and on the ES modules changes (import vs `require).

My gut tells me it's because everything is imported at the top in index.js, even if unneeded. Maybe we can use dynamic imports to import modules only when needed?

That could very well be the case! It would be great if we could try that.

triallax · 2022-05-27T19:47:19Z

No, right. But it does depend on the PureScript 0.15 changes and dependency upgrades (leading to faster parsing speed) and on the ES modules changes (import vs require).

Yeah, exactly; I just wanted to make sure we're on the same page.

That could very well be the case! It would be great if we could try that.

Then I'll do that in this PR soon, but I'll be taking a break from GitHub tomorrow and possibly the day after, so you'll have to wait some. :)

sharkdp · 2022-05-27T19:55:56Z

Then I'll do that in this PR soon, but I'll be taking a break from GitHub tomorrow and possibly the day after, so you'll have to wait some. :)

Of course. Please note that on my projects, there is certainly never any time pressure - and you don't have to explain if there are any "delays".. however long they might be.

Speaking of... would you be interested in joining me as a maintainer on this project? There are absolutely no obligations that come with this "role". I would add you to a "Maintainers" list in the README and give you admin permissions for this repo (and maybe purescript-quantities?). But only if you are interested.

triallax · 2022-05-27T20:09:11Z

Please note that on my projects, there is certainly never any time pressure - and you don't have to explain if there are any "delays".. however long they might be.

Okay, thanks, I understand that.

Speaking of... would you be interested in joining me as a maintainer on this project? There are absolutely no obligations that come with this "role".

Sure, I'm honored to take on that "role." :D

I would add you to a "Maintainers" list in the README and give you admin permissions for this repo (and maybe purescript-quantities?). But only if you are interested.

Yeah, I am interested in being a maintainer of Insect and purescript-quantities. I might also add purescript-decimals to that list, but reluctantly, as I don't think there's much work to do on it. If you want to add me as a maintainer to purescript-decimals too though, then I'm also okay with that.

sharkdp · 2022-05-27T20:40:10Z

Yeah, I am interested in being a maintainer of Insect and purescript-quantities. I might also add purescript-decimals to that list, but reluctantly, as I don't think there's much work to do on it. If you want to add me as a maintainer to purescript-decimals too though, then I'm also okay with that.

Awesome! Welcome on board. For purescript-decimals, we also have @thomashoneyman as a collaborator/maintainer, but I invited you as well.

triallax · 2022-05-27T20:42:11Z

Awesome! Welcome on board.

Pleasure to work with you!

For purescript-decimals, we also have @thomashoneyman as a collaborator/maintainer, but I invited you as well.

Ah, I wasn't aware of that.

triallax · 2022-06-02T15:39:58Z

Here are the results with index.js using dynamic imports (i.e. the PR's current state):

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`Startup time (insect '1+1')`	482.7 ± 5.3	473.0	489.7	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`Startup time, stdin mode (insect < computations-1.ins)`	490.5 ± 6.7	479.6	499.4	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`Evaluation speed (insect < computations-160.ins)`	1433.3 ± 44.1	1388.9	1533.6	1.00

Here are the results with the same changes as this PR, but without dynamic imports in index.js:

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`Startup time (insect '1+1')`	521.8 ± 9.7	507.5	542.3	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`Startup time, stdin mode (insect < computations-1.ins)`	522.7 ± 3.9	513.7	527.5	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`Evaluation speed (insect < computations-160.ins)`	1455.2 ± 9.3	1444.4	1473.2	1.00

Here are the results on master:

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`Startup time (insect '1+1')`	347.5 ± 6.7	342.1	361.2	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`Startup time, stdin mode (insect < computations-1.ins)`	359.9 ± 8.9	349.4	372.3	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`Evaluation speed (insect < computations-160.ins)`	2000.1 ± 19.2	1981.2	2042.7	1.00

As you can see, dynamic imports definitely help things, but they don't get us even close to the performance of master, which means that the major bottleneck (or bottlenecks) lies somewhere other than index.js itself.

What shall we do now? Dig deeper into the cause(s)?

P.S. I used Promise.all since it seemed to improve the performance a bit.

sharkdp · 2022-06-04T11:46:17Z

What shall we do now? Dig deeper into the cause(s)?

I would appreciate that. But I'm okay with postponing this task and merging this for now. Definitely looking forward to all the other improvements in this PR! Let me know what you prefer.

triallax · 2022-06-06T11:44:49Z

@sharkdp I'd prefer to merge this first so that we can be up-to-date with our dependencies, and then we can open an issue for the startup time regression. I just want to make an extra couple small changes to the PR and then we can go ahead and merge it.

sharkdp · 2022-06-06T12:28:11Z

@sharkdp I'd prefer to merge this first so that we can be up-to-date with our dependencies, and then we can open an issue for the startup time regression. I just want to make an extra couple small changes to the PR and then we can go ahead and merge it.

Sounds good!

triallax · 2022-06-07T18:04:09Z

src/Insect/Parser.purs

@@ -236,6 +237,7 @@ normalUnitDict = Dictionary
  , Q.fortnight ==> ["fortnights", "fortnight"]
  , Q.month ==> ["months", "month"]
  , Q.year ==> ["years", "year"]
+  , Q.julianYear ==> ["julianYears", "julianYear"]


I initially added julianyear and julianyears to this list, but the unit is "prettified" in the REPL as julianYear, and having both julianyear and julianYear is a very bad experience for completion.

triallax · 2022-06-07T18:08:44Z

I think I've done everything now, so you can go ahead and review/merge this PR.

You may wonder why all these changes are done in one commit, and that's because they're all kind of interrelated: - I initially updated PureScript to 0.15.x, which produces code that uses ES modules instead of CommonJS ones - The version of `purescript-decimals` in the 0.15.x package set was tested with decimal.js 10.3.1, so use that - Since we're using ES modules now, I updated `clipboardy` and `xdg-basedir` to versions that use ES modules (I admit that I didn't have to do this one in this commit) - `pulp browserify` doesn't work with PureScript 0.15.x, so I had to migrate to something else (I chose `esbuild`) Side note: `purescript-parsing` 9.0.0 brings with it a wonderful performance boost[1], which speeds up the test suite by 3 times on my machine (from `102.231 s ± 0.485 s` to `34.666 s ± 0.299 s`). [1]: purescript-contrib/purescript-parsing#154

sharkdp

Awesome, thank you very much.

triallax marked this pull request as draft May 27, 2022 10:13

triallax force-pushed the purescript-0.15.0 branch from 073704b to b24db9b Compare May 27, 2022 11:43

triallax mentioned this pull request May 27, 2022

Improving Insect's performance #262

Closed

triallax marked this pull request as ready for review May 27, 2022 16:10

triallax marked this pull request as draft June 2, 2022 15:24

triallax force-pushed the purescript-0.15.0 branch 2 times, most recently from 73323c7 to 1cb444d Compare June 2, 2022 15:28

triallax mentioned this pull request Jun 6, 2022

Update to PureScript Quantities v12.0.1 #308

Closed

triallax force-pushed the purescript-0.15.0 branch from 1cb444d to a60b461 Compare June 7, 2022 18:02

triallax commented Jun 7, 2022

View reviewed changes

triallax marked this pull request as ready for review June 7, 2022 18:08

triallax added 2 commits June 9, 2022 15:45

Add Julian year

2090a7d

triallax force-pushed the purescript-0.15.0 branch from a60b461 to 2090a7d Compare June 9, 2022 12:46

sharkdp approved these changes Jun 9, 2022

View reviewed changes

sharkdp merged commit 757624c into sharkdp:master Jun 9, 2022

triallax deleted the purescript-0.15.0 branch June 10, 2022 09:38

triallax mentioned this pull request Jun 10, 2022

Investigate and fix startup time increase #310

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update dependencies and use esbuild for bundling #303

Update dependencies and use esbuild for bundling #303

triallax commented May 27, 2022 •

edited

Loading

sharkdp commented May 27, 2022

sharkdp commented May 27, 2022

triallax commented May 27, 2022

sharkdp commented May 27, 2022

triallax commented May 27, 2022

triallax commented May 27, 2022

sharkdp commented May 27, 2022

triallax commented May 27, 2022

sharkdp commented May 27, 2022

triallax commented May 27, 2022

sharkdp commented May 27, 2022

triallax commented May 27, 2022

triallax commented Jun 2, 2022 •

edited

Loading

sharkdp commented Jun 4, 2022

triallax commented Jun 6, 2022 •

edited

Loading

sharkdp commented Jun 6, 2022

triallax Jun 7, 2022

triallax commented Jun 7, 2022

sharkdp left a comment

Update dependencies and use esbuild for bundling #303

Update dependencies and use esbuild for bundling #303

Conversation

triallax commented May 27, 2022 • edited Loading

sharkdp commented May 27, 2022

sharkdp commented May 27, 2022

triallax commented May 27, 2022

sharkdp commented May 27, 2022

master

this branch

triallax commented May 27, 2022

triallax commented May 27, 2022

sharkdp commented May 27, 2022

triallax commented May 27, 2022

sharkdp commented May 27, 2022

triallax commented May 27, 2022

sharkdp commented May 27, 2022

triallax commented May 27, 2022

triallax commented Jun 2, 2022 • edited Loading

sharkdp commented Jun 4, 2022

triallax commented Jun 6, 2022 • edited Loading

sharkdp commented Jun 6, 2022

triallax Jun 7, 2022

Choose a reason for hiding this comment

triallax commented Jun 7, 2022

sharkdp left a comment

Choose a reason for hiding this comment

triallax commented May 27, 2022 •

edited

Loading

triallax commented Jun 2, 2022 •

edited

Loading

triallax commented Jun 6, 2022 •

edited

Loading