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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request to have memory_pages (minimum page size) be configurable #56

Closed
JakeChampion opened this issue Jul 31, 2021 · 13 comments
Closed

Comments

@JakeChampion
Copy link
Contributor

JakeChampion commented Jul 31, 2021

Thank you for building Viceroy, this is such a great project - I really really like that it is now possible to run Fastly code locally 馃帀

I've been working on porting https://polyfill.io (a Fastly sponsored project) to be fully in Compute@Edge using the JavaScript SDK.

As of right now, due to the current lack of Edge Dictionary support in Viceroy (#11), I've inlined all the polyfills into the C@E code, this makes the bundled JavaScript file become 75.8 MB. After compiling the bundled JavaScript file into the Compute@Edge WASM file, it becomes 614 MB.

When trying to run this WASM file using Viceroy, an error is thrown:

Jul 31 20:33:53.927 ERROR memory index 0 has a minimum page size of 9469 which exceeds the limit of 2048

Is the limit of 2048 a hard limit of the Fastly Compute@Edge platform?
Would it be possible to have this limit raised of be configurable within Viceroy?

Update: I cloned Viceroy and changed the minimum page size from 2,048 to 10,240, when running Viceroy with the wasm file it works and it used 620.6 MB of WebAssembly heap

@aturon
Copy link
Contributor

aturon commented Aug 3, 2021

Thanks for the report!

C@E currently has a heap limit of 128MB / request, so I worry that this overall approach is not going to fly. Can you elaborate a bit on how the edge dictionary and polyfills relate for what you're doing? Maybe we can find an alternative approach.

@JakeChampion
Copy link
Contributor Author

The service takes a http request which contains a set of browser features to provide polyfills for and responds with a javascript file which bundles all the polyfills together.

My approach for moving this project to compute@edge is to store all the polyfills as individual items in an edge dictionary and then read from the dictionary all of the polyfills that were requested within the http request. This looks to work when deployed to compute@edge but I can't run it locally due to the lack of dictionary support in Viceroy at the moment.

In my eagerness to get something running locally I replaced the dictionary solution and instead inlined all the polyfills, which I know is not something that would work in production but it did mean I could then get it running locally using Viceroy which drastically sped up my development workflow

I'm happy to wait for the dictionary support to arrive in Viceroy.

@peterbourgon
Copy link

As an interim solution, you could build a small Edge Dictionary adapter, which used a mock implementation that read from e.g. files on disk when running in Viceroy, and the real thing otherwise.

@pchickey
Copy link
Contributor

pchickey commented Aug 5, 2021

Viceroy does not support reading files from disk

@peterbourgon
Copy link

peterbourgon commented Aug 5, 2021

Aw beans. An Edge-Dictionary-alike microservice modeled as a backend? I'm flailing here.

@dkegel-fastly
Copy link
Contributor

I understand #11 has a champion already :-). https://github.com/JakeChampion/Viceroy/pull/2

@JakeChampion
Copy link
Contributor Author

JakeChampion commented Aug 8, 2021

@dkegel-fastly quite possibly. I wanted to give it a go and see if I could figure out why my edge dictionaries were not working when deployed to the C@E platform, I wanted to see if my JavaScript code would work with Viceroy if I added dictionary support. It turns out that there is a potential bug/gotcha with wizer and pre-initialising any objects which rely on platform specific values such as edge/local dictionaries. Wizer doesn't know about the platform and defaults to setting invalid handle values for the objects -- fastly/js-compute-runtime#3

I've got a working implementation of local dictionary support now, it was fun to work on this and learn how viceroy, js-compute-runtime, and wizer all fit together.

The implementation reads a new section from the local_server part of fastly.toml named dictionaries.

The local_server.dictionaries section is an array of tables with two fields: name, and file.

The name field is a string which is used as the name of the dictionary and is used via the Dictionary.open method defined in the compute-at-edge.witx file. This field uses the same validation rules as the Fastly API (Name must start with alphabetical and contain only alphanumeric, underscore, and whitespace).

The file field is a string which is a file-path pointing to a JSON file which contains all the dictionary items within an object.

An example of a full local_server section of fastly.toml now looks like this:

[local_server]
  [local_server.backends]
    [local_server.backends.dog]
    url = "http://localhost:7878/dog-mocks"
  [local_server.dictionaries]
    [local_server.dictionaries.polyfills]
    name="polyfills"
    file="./polyfills.json"

Update: With the local dictionary support, Viceroy can now run the service locally and requests for all the polyfills complete using 7.1 MB of WebAssembly heap

@JakeChampion
Copy link
Contributor Author

Should I open a pull-request for my implementation of local dictionary support?

@cratelyn
Copy link
Contributor

cratelyn commented Aug 9, 2021

Should I open a pull-request for my implementation of local dictionary support?

That is amazing work @JakeChampion! A PR would be quite welcome, I would be happy to review that work. 馃

@JakeChampion
Copy link
Contributor Author

Should I open a pull-request for my implementation of local dictionary support?

That is amazing work @JakeChampion! A PR would be quite welcome, I would be happy to review that work. 馃

Here it is 鈽猴笍 -- #61

@mgattozzi
Copy link
Contributor

Just want to say @JakeChampion thank you for taking the time to do this. It's definitely appreciated 馃槃

@JakeChampion
Copy link
Contributor Author

I'll close this issue as the ability to have local dictionary support in viceroy has been added and solves the original issue I was having.

Thank you all for Viceroy, it is so good being able to run a Fastly Service's code locally 鈽猴笍

@cratelyn
Copy link
Contributor

and thank you for the dictionary support @JakeChampion! 馃帀 馃摉

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

No branches or pull requests

7 participants