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

Reduce visit recursion for large binary expressions? #9122

Open
dsherret opened this issue Jul 2, 2024 · 5 comments
Open

Reduce visit recursion for large binary expressions? #9122

dsherret opened this issue Jul 2, 2024 · 5 comments
Assignees
Milestone

Comments

@dsherret
Copy link
Contributor

dsherret commented Jul 2, 2024

Describe the feature

In certain transforms, it might be good to not use recursion. For example, it's fairly common for people to write code like the following:

const data = "AAEAAAASAQAABAAgR0RFRrRCsIIAAir4AAACYkdQT1Or8IZpAAItXAAAZS5H" +
  "U1VCeoGFdwACkowAABWQT1MvMpl2sYAAAhh4AAAAYGNtYXDG7lFtAAId8AAA" +
  "....etc for many many lines...";

It might be good for transforms like the resolver to special case these scenarios in its transformers and instead of visiting the expressions recursively it could visit them one by one in a loop so that it doesn't overflow the stack.

Babel plugin or link to the feature description

No response

Additional context

denoland/deno#24325 (comment)

@kdy1 kdy1 added this to the Planned milestone Jul 2, 2024
@kdy1 kdy1 self-assigned this Jul 2, 2024
@kdy1
Copy link
Member

kdy1 commented Jul 3, 2024

Are you fine with a solution based on stacker? (Dynamically sized stacks)
It's a general solution so it can be applied to any visitor quickly, but it may not work on some platforms.

Also, can you list the SWC passes used by Deno? IIRC, I added an in-tree testing for transforms used by a team, but I'm not sure if it was Deno or Parcel. I'll add one more in-tree test suite for it and for stack size.

@mischnic
Copy link
Contributor

mischnic commented Jul 3, 2024

An older issue for the same problem: #1627

@dsherret
Copy link
Contributor Author

dsherret commented Jul 3, 2024

Also, can you list the SWC passes used by Deno? IIRC, I added an in-tree testing for transforms used by a team, but I'm not sure if it was Deno or Parcel. I'll add one more in-tree test suite for it and for stack size.

Nope, wasn't us.

Are you fine with a solution based on stacker? (Dynamically sized stacks)
It's a general solution so it can be applied to any visitor quickly, but it may not work on some platforms.

I think it's good to use stacker generally, but I feel like scenarios like this could be optimized to not use so much stack? It will probably run a lot faster.

Also, can you list the SWC passes used by Deno?

let mut passes = chain!(
    resolver(unresolved_mark, top_level_mark, true),
    proposal::decorator_2022_03::decorator_2022_03(),
    proposal::explicit_resource_management::explicit_resource_management(),
    helpers::inject_helpers(top_level_mark),
    typescript::typescript(typescript::Config {
      verbatim_module_syntax: false,
      import_not_used_as_values: typescript::ImportsNotUsedAsValues::Remove,
      no_empty_export: true,
      import_export_assign_config: typescript::TsImportExportAssignConfig::Preserve,
      ts_enum_is_mutable: true,
    }, top_level_mark),
    fixer(Some(comments)),
    hygiene(),
);

@kdy1
Copy link
Member

kdy1 commented Jul 4, 2024

Did you enable swc_ecma_utils/stacker? All of those passes should already have dynamic stack size support.

@kdy1
Copy link
Member

kdy1 commented Jul 4, 2024

Deno isn't using it. https://github.com/denoland/deno/blob/147411e64b22fe74cb258125acab83f9182c9f81/Cargo.lock#L6544-L6561

It should have stacker entry if it's enabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants