Skip to content

feat: Compiler optimisations - #1053

Merged
josephjclark merged 10 commits into
release/nextfrom
ignore-top-object-add-import
Oct 10, 2025
Merged

josephjclark merged 10 commits into
release/nextfrom
ignore-top-object-add-import

Conversation

@doc-han

@doc-han doc-han commented Oct 8, 2025 •

Copy link
Copy Markdown
Collaborator

Short Description

When running findAllDanglingIdentifiers in the add-imports transform, we want to ignore top-level object declarations.

Fixes #1029

Implementation Details

A more detailed breakdown of the changes, including motivations (if not provided in the issue).

QA Notes

List any considerations/cases/advice for testing/QA here.

AI Usage

Please disclose how you've used AI in this work (it's cool, we just want to know!):

  • Code generation (copilot but not intellisense)
  • Learning or fact checking
  • Strategy / design
  • Optimisation / refactoring
  • Translation / spellchecking / doc gen
  • Other
  • I have not used AI

You can read more details in our Responsible AI Policy

Release branch checklist

Delete this section if this is not a release PR.

If this IS a release branch:

  • Run pnpm changeset version from root to bump versions
  • Run pnpm install
  • Commit the new version numbers
  • Run pnpm changeset tag to generate tags
  • Push tags git push --tags

Tags may need updating if commits come in after the tags are first generated.

@github-project-automation github-project-automation Bot moved this to New Issues in Core Oct 8, 2025
@doc-han
doc-han requested a review from josephjclark October 8, 2025 14:04
@josephjclark

Copy link
Copy Markdown
Collaborator

Perfect, that's exactly what I'm expecting 👍

doc-han and others added 2 commits October 8, 2025 16:53
* feat: ignore objects for lazy state

* tests: ignore top level object in lazy state
@josephjclark
josephjclark changed the base branch from main to release/next October 8, 2025 17:44
@josephjclark

Copy link
Copy Markdown
Collaborator

Dang, something feels off here.

Running this against my (refactored) 1.6mb source file, I'm still seeing 230mb of memory get used up. I can see that the lazy state vistor is exiting early - but there's still too much tree traversal going on.

The changes here look good and I'm happy, but we'll need to investigate a bit more before we can declare victory on this particular problem :(

@josephjclark

josephjclark commented Oct 9, 2025 •

Copy link
Copy Markdown
Collaborator

Here's a much better trace output for parsing with the new changes:

[pre acorn parse] Used heap at 58.79mb
[post acorn parse] Used heap at 157.66mb

[transformer lazy-state] Used heap at 158.27mb
transformer lazy-state: 1.743ms

[transformer promises] Used heap at 202.67mb
transformer promises: 82.818ms

[transformer ensure-exports] Used heap at 202.74mb
transformer ensure-exports: 0.662ms

[transformer top-level-operations] Used heap at 249.66mb
transformer top-level-operations: 68.538ms

[transformer add-imports] Used heap at 249.69mb
transformer add-imports: 0.334ms

[finished] Used heap at 249.70mb

For the record, here's what it's like without the optimisation:

[pre acorn parse] Used heap at 60.44mb
[post acorn parse] Used heap at 154.56mb

[transformer lazy-state] Used heap at 201.88mb
transformer lazy-state: 91.469ms

[transformer promises] Used heap at 234.78mb
transformer promises: 99.671ms

[transformer ensure-exports] Used heap at 234.86mb
transformer ensure-exports: 0.67ms

[transformer top-level-operations] Used heap at 234.95mb
transformer top-level-operations: 0.486ms

[transformer add-imports] Used heap at 234.97mb
transformer add-imports: 0.196ms

[finished] Used heap at 234.97mb

I can see that lazy state and promises have improved - but promises and top level ops are still kinda expensive. You were talking about top level ops @doc-han maybe we do need to look if there's a quick win there

Use openfn compile --trace
@josephjclark

Copy link
Copy Markdown
Collaborator

@doc-han I think I can see two cheap fixes here:

  1. Flip top-level-operations to start from Program and do its own traversal - see compiler: optimise top level operations transformer #1055
  2. Do the same with promises as we've done for imports: visit call expressions and abort if they're at the top level

@josephjclark josephjclark changed the title feat: ignore top objects in add-import feat: Compiler optimisations Oct 9, 2025
@josephjclark
josephjclark merged commit 11e3e05 into release/next Oct 10, 2025
4 of 6 checks passed
@josephjclark
josephjclark deleted the ignore-top-object-add-import branch October 10, 2025 16:33
@github-project-automation github-project-automation Bot moved this from New Issues to Done in Core Oct 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Compiler optimisation: ignore top level declarations

2 participants