Use known and adaptive buffer sizes to lower generated garbage

[cee-dub]

Closes #101

To enable merging this branch, we verified which of the following hostcalls fully support adaptive buffers (return a buflen error with a usable value) in both ExecuteD and Viceroy:

ExecuteD:

• fastly_http_req method_get
• fastly_http_req uri_get
• fastly_dictionary get (partial!) Full support needs new hostcall that returns the needed size
• fastly_geo lookup
• fastly_secret_store plaintext
• fastly_device_detection lookup
• fastly_cache get_user_metadata
• fastly_http_req downstream_tls_client_hello

Viceroy:

• fastly_device_detection lookup
• fastly_dictionary get (partial!) Full support needs new hostcall that returns the needed size
• fastly_http_req method_get
• fastly_http_req uri_get
• fastly_geo lookup
• fastly_secret_store plaintext
• fastly_cache get_user_metadata (❌ not implemented in viceroy)
• fastly_http_req downstream_tls_client_hello (❌ not implemented in viceroy)

Any hostcalls found not to match behavior between ExecuteD and Viceroy will be corrected in Viceroy.

Any hostcalls found not to return a useable value in ExecuteD will be held back in a separate branch until a new version is fully rolled to the fleet.

[joeshaw]

This mostly looks good to me, thanks for taking it on. I have a few minor comments below.

[joeshaw]

This would address #101 btw.

[joeshaw]

Due to the failing device detection test, there's something not working as expected with adaptive buffers.

Specifically: I thought that buf.NValue() would report the size of the buffer needed to hold the full value, if the passed buffer was too short. Instead it always equals 0.

It's possible the device detection code doesn't support adaptive buffers. Probably an oversight.

[joeshaw]

Due to the failing device detection test, there's something not working as expected with adaptive buffers.
Specifically: I thought that buf.NValue() would report the size of the buffer needed to hold the full value, if the passed buffer was too short. Instead it always equals 0.

It's possible the device detection code doesn't support adaptive buffers. Probably an oversight.

Looks like a bug in Viceroy: https://github.com/fastly/Viceroy/blob/main/lib/src/wiggle_abi/device_detection_impl.rs#L46-L51

Note how n_written is not set to the expected size of the buffer.

Contrast this with the equivalent secret store code: https://github.com/fastly/Viceroy/blob/main/lib/src/wiggle_abi/secret_store_impl.rs#L105-L114

It doesn't appear to be an issue with the production Compute platform though.

[joeshaw]

fastly/Viceroy#383 adds adaptive buffer support for geolocation and device detection to Viceroy, which should allow these tests to pass once it's merged and a new Viceroy is released.

[lpereira]

I like these changes! From a cursory glance I didn't see anything obviously wrong, just the minor nits I pointed above.

[elliottt]

I've verified on the ExecuteD side that all the host calls (except for fastly_dictionary#get) will populate the length needed outparam. Additionally, the two Viceroy host calls that you mentioned being unsure about (fastly_cache#get_user_metadata and fastly_http_req#downstream_tls_client_hello) are both currently unimplemented.