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.

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

Investigate and fix startup time increase #310

Closed
triallax opened this issue Jun 10, 2022 · 2 comments · Fixed by #322
Closed

Investigate and fix startup time increase #310

triallax opened this issue Jun 10, 2022 · 2 comments · Fixed by #322
Assignees
Milestone

Comments

@triallax
Copy link
Contributor

#303 caused a pretty bad regression in startup times. We should investigate why the startup time increased and how to reduce it.

@triallax
Copy link
Contributor Author

I made index.js import a minified and bundled insect.js instead of output/Insect/index.js, and here are the benchmark results:

Command Mean [ms] Min [ms] Max [ms] Relative
Startup time (insect '1+1') 175.9 ± 1.7 172.2 179.5 1.00
Command Mean [ms] Min [ms] Max [ms] Relative
Startup time, stdin mode (insect < computations-1.ins) 194.9 ± 1.5 192.4 197.4 1.00
Command Mean [ms] Min [ms] Max [ms] Relative
Evaluation speed (insect < computations-160.ins) 1114.2 ± 13.1 1091.9 1131.6 1.00

Compared to master and #303 (see #303 (comment)), this is a HUGE improvement.

@sharkdp
Copy link
Owner

sharkdp commented Jun 12, 2022

Compared to master and #303 (see #303 (comment)), this is a HUGE improvement.

Oh cool!

@triallax triallax self-assigned this Jun 21, 2022
@triallax triallax added this to the v5.8.0 milestone Jul 20, 2022
triallax added a commit to triallax/insect that referenced this issue Jul 20, 2022
Fixes sharkdp#310, and provides a start for sharkdp#306.

This improves the startup time on my machine significantly. I'll let the
benchmark results speak for themselves:

Before commit:

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Startup time (insect '1+1')` | 513.9 ± 24.4 | 496.4 | 581.2 | 1.00 |

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Startup time, stdin mode (insect < computations-1.ins)` | 535.5 ± 14.2 | 524.6 | 572.7 | 1.00 |

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Evaluation speed (insect < computations-160.ins)` | 1502.1 ± 42.4 | 1466.2 | 1580.7 | 1.00 |

After commit:

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Startup time (insect '1+1')` | 192.4 ± 18.5 | 185.4 | 259.1 | 1.00 |

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Startup time, stdin mode (insect < computations-1.ins)` | 208.1 ± 2.0 | 204.8 | 212.2 | 1.00 |

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Evaluation speed (insect < computations-160.ins)` | 1153.0 ± 17.6 | 1136.6 | 1198.3 | 1.00 |

Also, this commit causes `output` to not be published to NPM, which
makes for a much installation size: Insect 5.7.0 on NPM has an unpacked
size of 29.2 MBs, but `npm publish --dry-run` with this commit reports
378.8 kB. `.npmignore` was also removed, as the `files` property in
`package.json` includes only the files we want, so `.npmignore` does
basically nothing. Read
https://docs.npmjs.com/cli/v8/configuring-npm/package-json/#files for
more information.

This commit additionally makes a small modification to `index.js` to
make it work with Node 12. Unfortunately, at least one of our
dependencies (`clipboardy`) does not support Node 10, so I could not
make `index.js` work with Node 10.
triallax added a commit to triallax/insect that referenced this issue Jul 20, 2022
Fixes sharkdp#310, and provides a start for sharkdp#306.

This improves the startup time on my machine significantly. I'll let the
benchmark results speak for themselves:

Before commit:

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Startup time (insect '1+1')` | 513.9 ± 24.4 | 496.4 | 581.2 | 1.00 |

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Startup time, stdin mode (insect < computations-1.ins)` | 535.5 ± 14.2 | 524.6 | 572.7 | 1.00 |

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Evaluation speed (insect < computations-160.ins)` | 1502.1 ± 42.4 | 1466.2 | 1580.7 | 1.00 |

After commit:

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Startup time (insect '1+1')` | 192.4 ± 18.5 | 185.4 | 259.1 | 1.00 |

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Startup time, stdin mode (insect < computations-1.ins)` | 208.1 ± 2.0 | 204.8 | 212.2 | 1.00 |

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Evaluation speed (insect < computations-160.ins)` | 1153.0 ± 17.6 | 1136.6 | 1198.3 | 1.00 |

Also, this commit causes `output` to not be published to NPM, which
makes for a much smaller installation size: Insect 5.7.0 on NPM has an
unpacked size of 29.2 MBs, but `npm publish --dry-run` with this commit
reports 378.8 kB. `.npmignore` was also removed, as the `files` property
in `package.json` includes only the files we want, so `.npmignore` does
basically nothing. Read
https://docs.npmjs.com/cli/v8/configuring-npm/package-json/#files for
more information.

This commit additionally makes a small modification to `index.js` to
make it work with Node 12. Unfortunately, at least one of our
dependencies (`clipboardy`) does not support Node 10, so I could not
make `index.js` work with Node 10.
triallax added a commit to triallax/insect that referenced this issue Jul 24, 2022
Fixes sharkdp#310, and provides a start for sharkdp#306.

This improves the startup time on my machine significantly. I'll let the
benchmark results speak for themselves:

Before commit:

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Startup time (insect '1+1')` | 513.9 ± 24.4 | 496.4 | 581.2 | 1.00 |

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Startup time, stdin mode (insect < computations-1.ins)` | 535.5 ± 14.2 | 524.6 | 572.7 | 1.00 |

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Evaluation speed (insect < computations-160.ins)` | 1502.1 ± 42.4 | 1466.2 | 1580.7 | 1.00 |

After commit:

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Startup time (insect '1+1')` | 192.4 ± 18.5 | 185.4 | 259.1 | 1.00 |

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Startup time, stdin mode (insect < computations-1.ins)` | 208.1 ± 2.0 | 204.8 | 212.2 | 1.00 |

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Evaluation speed (insect < computations-160.ins)` | 1153.0 ± 17.6 | 1136.6 | 1198.3 | 1.00 |

Also, this commit causes `output` to not be published to NPM, which
makes for a much smaller installation size: Insect 5.7.0 on NPM has an
unpacked size of 29.2 MBs, but `npm publish --dry-run` with this commit
reports 378.8 kB. `.npmignore` was also removed, as the `files` property
in `package.json` includes only the files we want, so `.npmignore` does
basically nothing. Read
https://docs.npmjs.com/cli/v8/configuring-npm/package-json/#files for
more information.

This commit additionally makes a small modification to `index.js` to
make it work with Node 12. Unfortunately, at least one of our
dependencies (`clipboardy`) does not support Node 10, so I could not
make `index.js` work with Node 10.
sharkdp pushed a commit that referenced this issue Jul 26, 2022
Fixes #310, and provides a start for #306.

This improves the startup time on my machine significantly. I'll let the
benchmark results speak for themselves:

Before commit:

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Startup time (insect '1+1')` | 513.9 ± 24.4 | 496.4 | 581.2 | 1.00 |

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Startup time, stdin mode (insect < computations-1.ins)` | 535.5 ± 14.2 | 524.6 | 572.7 | 1.00 |

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Evaluation speed (insect < computations-160.ins)` | 1502.1 ± 42.4 | 1466.2 | 1580.7 | 1.00 |

After commit:

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Startup time (insect '1+1')` | 192.4 ± 18.5 | 185.4 | 259.1 | 1.00 |

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Startup time, stdin mode (insect < computations-1.ins)` | 208.1 ± 2.0 | 204.8 | 212.2 | 1.00 |

| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|:---|---:|---:|---:|---:|
| `Evaluation speed (insect < computations-160.ins)` | 1153.0 ± 17.6 | 1136.6 | 1198.3 | 1.00 |

Also, this commit causes `output` to not be published to NPM, which
makes for a much smaller installation size: Insect 5.7.0 on NPM has an
unpacked size of 29.2 MBs, but `npm publish --dry-run` with this commit
reports 378.8 kB. `.npmignore` was also removed, as the `files` property
in `package.json` includes only the files we want, so `.npmignore` does
basically nothing. Read
https://docs.npmjs.com/cli/v8/configuring-npm/package-json/#files for
more information.

This commit additionally makes a small modification to `index.js` to
make it work with Node 12. Unfortunately, at least one of our
dependencies (`clipboardy`) does not support Node 10, so I could not
make `index.js` work with Node 10.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants