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

Replace the question mark operator with if let Some else return None #4

Closed
wants to merge 1 commit into from

Conversation

bjorn3
Copy link

@bjorn3 bjorn3 commented Nov 5, 2021

This reduces the amount of function calls performed in debug mode and reduces the amount of time the optimizer takes to run. While it may make the generated code a bit uglier, It is a very simple change in the codegen code and doesn't make the generated code unreadable.

This reduces the amount of function calls performed in debug mode and
reduces the amount of time the optimizer takes to run. While it may make
the generated code a bit uglier, It is a very simple change in the
codegen code and doesn't make the generated code unreadable.
@bjorn3
Copy link
Author

bjorn3 commented Nov 6, 2021

Ping @fitzgen (seems you aren't watching your fork)

@fitzgen
Copy link
Owner

fitzgen commented Nov 8, 2021

I'd like to reiterate the principle that @cfallin stated here: we shouldn't be optimizing for the performance of non-optimized builds, especially at the cost of how easy it is to read/debug the code. This adds significant noise to the generated code (take a look at its diff).

As @sunfishcode said, once we settle into ISLE more, and have worked out all the kinks / generally aren't debugging lowering anymore, we can revisit some of these points. But for now we should optimize for debugging/readability and generating code that is fast when run through LLVM's optimization passes.

@fitzgen fitzgen closed this Nov 8, 2021
fitzgen added a commit that referenced this pull request Nov 15, 2021
@bjorn3 bjorn3 deleted the cheaper-isle branch April 7, 2023 11:47
fitzgen pushed a commit that referenced this pull request Apr 21, 2023
* Integrate experimental HTTP into wasmtime.

* Reset Cargo.lock

* Switch to bail!, plumb options partially.

* Implement timeouts.

* Remove generated files & wasm, add Makefile

* Remove generated code textfile

* Update crates/wasi-http/Cargo.toml

Co-authored-by: Eduardo de Moura Rodrigues <16357187+eduardomourar@users.noreply.github.com>

* Update crates/wasi-http/Cargo.toml

Co-authored-by: Eduardo de Moura Rodrigues <16357187+eduardomourar@users.noreply.github.com>

* Extract streams from request/response.

* Fix read for len < buffer length.

* Formatting.

* types impl: swap todos for traps

* streams_impl: idioms, and swap todos for traps

* component impl: idioms, swap all unwraps for traps, swap all todos for traps

* http impl: idiom

* Remove an unnecessary mut.

* Remove an unsupported function.

* Switch to the tokio runtime for the HTTP request.

* Add a rust example.

* Update to latest wit definition

* Remove example code.

* wip: start writing a http test...

* finish writing the outbound request example

havent executed it yet

* better debug output

* wasi-http: some stubs required for rust rewrite of the example

* add wasi_http tests to test-programs

* CI: run the http tests

* Fix some warnings.

* bump new deps to latest releases (#3)

* Add tests for wasi-http to test-programs (#2)

* wip: start writing a http test...

* finish writing the outbound request example

havent executed it yet

* better debug output

* wasi-http: some stubs required for rust rewrite of the example

* add wasi_http tests to test-programs

* CI: run the http tests

* bump new deps to latest releases

h2 0.3.16
http 0.2.9
mio 0.8.6
openssl 0.10.48
openssl-sys 0.9.83
tokio 1.26.0

---------

Co-authored-by: Brendan Burns <bburns@microsoft.com>

* Update crates/test-programs/tests/http_tests/runtime/wasi_http_tests.rs

* Update crates/test-programs/tests/http_tests/runtime/wasi_http_tests.rs

* Update crates/test-programs/tests/http_tests/runtime/wasi_http_tests.rs

* wasi-http: fix cargo.toml file and publish script to work together (#4)

unfortunately, the publish script doesn't use a proper toml parser (in
order to not have any dependencies), so the whitespace has to be the
trivial expected case.

then, add wasi-http to the list of crates to publish.

* Update crates/test-programs/build.rs

* Switch to rustls

* Cleanups.

* Merge switch to rustls.

* Formatting

* Remove libssl install

* Fix tests.

* Rename wasi-http -> wasmtime-wasi-http

* prtest:full

Conditionalize TLS on riscv64gc.

* prtest:full

Fix formatting, also disable tls on s390x

* prtest:full

Add a path parameter to wit-bindgen, remove symlink.

* prtest:full

Fix tests for places where SSL isn't supported.

* Update crates/wasi-http/Cargo.toml

---------

Co-authored-by: Eduardo de Moura Rodrigues <16357187+eduardomourar@users.noreply.github.com>
Co-authored-by: Pat Hickey <phickey@fastly.com>
Co-authored-by: Pat Hickey <pat@moreproductive.org>
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.

2 participants