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

🐛 Avoid recursion in generators due to no tail-call optimizations #739

Merged
merged 1 commit into from Jan 24, 2022

Conversation

wwilsman
Copy link
Contributor

What is this?

The check within the waitFor util was originally created as a recursive function. With recent changes, the util was converted to make use of the cancelability of generators. However, generators cannot be tail-call optimized like the previous implementation was. This is due to technical reasons within V8 around the nature of how generator's next() function handles script evaluation.

Because the waitFor util polls and may run for a long time, the lack of tail-call optimization caused the call stack to continuously grow the longer the util took to resolve. Eventually this could/will result in call stack errors (#735).

This PR refactors the generator within the util to use a while (true) loop rather than a recursive yield*. The infinite loop is broken if the predicate passes at least once and at least one more time if idle is provided.

@wwilsman wwilsman added the 🐛 bug Something isn't working label Jan 24, 2022
@wwilsman wwilsman enabled auto-merge (squash) January 24, 2022 19:33
Copy link
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is due to technical reasons within V8 around the nature of how generator's next() function handles script evaluation.

Ohhhhhhhhhh man, yeah here we are lol

🏁

@wwilsman wwilsman merged commit 2e678a7 into master Jan 24, 2022
@wwilsman wwilsman deleted the ww/fix-wait-for-callstack branch January 24, 2022 19:40
bors added a commit to rust-lang/crates.io that referenced this pull request Jan 28, 2022
Update dependency @percy/cli to v1.0.0-beta.74

[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [`@percy/cli](https://togithub.com/percy/cli)` | [`1.0.0-beta.73` -> `1.0.0-beta.74`](https://renovatebot.com/diffs/npm/`@percy%2fcli/1.0.0-beta.73/1.0.0-beta.74)` | [![age](https://badges.renovateapi.com/packages/npm/`@percy%2fcli/1.0.0-beta.74/age-slim)](https://docs.renovatebot.com/merge-confidence/)` | [![adoption](https://badges.renovateapi.com/packages/npm/`@percy%2fcli/1.0.0-beta.74/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)` | [![passing](https://badges.renovateapi.com/packages/npm/`@percy%2fcli/1.0.0-beta.74/compatibility-slim/1.0.0-beta.73)](https://docs.renovatebot.com/merge-confidence/)` | [![confidence](https://badges.renovateapi.com/packages/npm/`@percy%2fcli/1.0.0-beta.74/confidence-slim/1.0.0-beta.73)](https://docs.renovatebot.com/merge-confidence/)` |

---

### Release Notes

<details>
<summary>percy/cli</summary>

### [`v1.0.0-beta.74`](https://togithub.com/percy/cli/releases/v1.0.0-beta.74)

[Compare Source](https://togithub.com/percy/cli/compare/v1.0.0-beta.73...v1.0.0-beta.74)

<!-- Release notes generated using configuration in .github/release.yml at master -->

#### What's Changed

##### 🐛 Bug Fixes

-   🐛 Avoid recursion in generators due to no tail-call optimizations by [`@&#8203;wwilsman](https://togithub.com/wwilsman)` in [percy/cli#739

##### ⬆️⬇️ Dependency Updates

-   ⬆️ Bump shelljs from 0.8.4 to 0.8.5 by [`@&#8203;dependabot](https://togithub.com/dependabot)` in [percy/cli#722
-   ⬆️ Bump [`@&#8203;babel/preset-env](https://togithub.com/babel/preset-env)` from 7.16.7 to 7.16.8 by [`@&#8203;dependabot](https://togithub.com/dependabot)` in [percy/cli#723
-   ⬆️ Bump ws from 8.4.0 to 8.4.2 by [`@&#8203;dependabot](https://togithub.com/dependabot)` in [percy/cli#729
-   ⬆️ Bump nock from 13.2.1 to 13.2.2 by [`@&#8203;dependabot](https://togithub.com/dependabot)` in [percy/cli#725
-   ⬆️ Bump log4js from 6.3.0 to 6.4.0 by [`@&#8203;dependabot](https://togithub.com/dependabot)` in [percy/cli#736

**Full Changelog**: percy/cli@v1.0.0-beta.73...v1.0.0-beta.74

</details>

---

### Configuration

📅 **Schedule**: "before 3am on Monday" (UTC).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/rust-lang/crates.io).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants