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

Agent: Bundle WASM artifacts #3386

Merged
merged 7 commits into from
Mar 15, 2024
Merged

Agent: Bundle WASM artifacts #3386

merged 7 commits into from
Mar 15, 2024

Conversation

philipp-spiess
Copy link
Contributor

@philipp-spiess philipp-spiess commented Mar 12, 2024

Closes #3302

This PR ensures that cody agent always has access to the WASM artifacts. While building this, we found out that it is necessary to bundle the wasm inside the binary:

----stderr----
Aborted(Error: File '/**/dist/tree-sitter.wasm' was not included into executable at compilation stage. Please recompile adding it as asset or script.)
--------------
----stderr----
failed to asynchronously prepare wasm: RuntimeError: Aborted(Error: File '/**/dist/tree-sitter.wasm' was not included into executable at compilation stage. Please recompile adding it as asset or script.). Build with -sASSERTIONS for more info.
Aborted(RuntimeError: Aborted(Error: File '/**/dist/tree-sitter.wasm' was not included into executable at compilation stage. Please recompile adding it as asset or script.). Build with -sASSERTIONS for more info.)
--------------
----stderr----
Uncaught exception: RuntimeError: Aborted(RuntimeError: Aborted(Error: File '/**/dist/tree-sitter.wasm' was not included into executable at compilation stage. Please recompile adding it as asset or script.). Build with -sASSERTIONS for more info.). Build with -sASSERTIONS for more info.
    at abort (/snapshot/dist/agent.js)
    at /snapshot/dist/agent.js
--------------

Since we have more than one agent build, this would result in a huge increase in bundle size. We offset that by adding gzip compression. I tried brotli but due to the nature of not having centralized builds yet, it's not worth the additional build time.

Here are the final bundle size changes (for the aarch build):

  • Currently the agent binaries are ~82mb
  • Without compression they would grow to almost ~146mb
  • With compression (gzip) ~64mb

Test plan

I added a new script that we can use to play around with the binary under agent/scripts/test-agent-binary.ts. You can use it to see that WASM works:

  • cd agent/
  • pnpm run build-agent-binaries
  • pnpm run test-agent-binary

This will output something like this. Notice the addition of the intent which requires wasm to load 😎

Screenshot 2024-03-13 at 13 06 28

@philipp-spiess philipp-spiess requested a review from a team March 13, 2024 12:06
@philipp-spiess philipp-spiess marked this pull request as ready for review March 13, 2024 12:06
"name": "@sourcegraph/cody-agent",
"name": "@sourcegraph/agent",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This rename is necessary so the build artifacts are still called agent- and not cody-agent-. We don't use that name anywhere though so it should be fine.

"latest-win-x64",
"latest-macos-arm64"
],
"assets": "dist/*.{wasm,map}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have to use to this pkg syntax and define the wasm artifact here so it'll be inlined properly.

@philipp-spiess philipp-spiess requested a review from a team March 15, 2024 14:23
name: 'defaultClient',
accessToken,
},
'./dist/agent-macos-arm64'
Copy link
Contributor

Choose a reason for hiding this comment

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

So that test will only work on windows? Can we pickup proper binary depending on the platform?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I'll copy paste the right version selector here if we ever need to debug on win envs.

Copy link
Contributor

@pkukielka pkukielka left a comment

Choose a reason for hiding this comment

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

LGTM

@philipp-spiess philipp-spiess enabled auto-merge (squash) March 15, 2024 16:28
@philipp-spiess philipp-spiess merged commit 3e40fb5 into main Mar 15, 2024
16 checks passed
@philipp-spiess philipp-spiess deleted the ps/agent-wasm branch March 15, 2024 17:00
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 this pull request may close these issues.

Cody Agent should always use WASM artifacts
2 participants