kilork / actix-web-static-files

actix-web static files as resources support
The Unlicense
75 stars 18 forks source link

Generate response at compile time? #17

Closed IronOxidizer closed 3 years ago

IronOxidizer commented 4 years ago

Is this project generating responses at compile time? It should certainly be possible as the only thing that changes is content length which is also determined at compile time.

kilork commented 4 years ago

We generate response using static string resources embedded in library.

We do not have anything outside, I developed this library because of the way Rust uses to distribute packages. The idea was to have single executable with all resource bundled inside. After this there is a trivial code which resolves url to static resource reference. We do not calculate or read anything. I want to do as much work as possible during compile time to not mess with it later.

I am not quite sure I understand a question. Can you explain a little bit?

IronOxidizer commented 4 years ago

We generate response using static string resources embedded in library. ... We do not calculate or read anything. I want to do as much work as possible during compile time to not mess with it later.

That answers my question. Thanks.

IronOxidizer commented 4 years ago

Would it be possible to do this once at the start of runtime and cache + reuse the response?

let mut resp = HttpResponse::build(StatusCode::OK);
        resp.set_header(header::CONTENT_TYPE, file.mime_type)
            .if_some(etag, |etag, resp| {
                resp.set(header::ETag(etag));
            });
        resp.set_body(file.data)

This would remove further allocations and checks that only need to be done once since the content is static. Ideally this would be done at compile time but I don't think it's possible.

kilork commented 4 years ago

It is interesting idea. For me from the beginning it was important to deliver resources inside of the binary. But because people take actix-web mostly because of performance we should keep eye on this.

To do this thing we are missing benchmark and memory consumption tests.

I actually tried some optimizations, but because of async / await nature of this ecosystem it is not always something easy to do. I think also our current generated function does not depend on actix types, means it is a breaking change.

Do you struggle with some performance issues? Have you compared performance with other solutions?

IronOxidizer commented 4 years ago

I don't have any performance issues, I just think it would be ideal to do it this way. After doing some more research, I've discovered that ResponseBuilder is non-reusable.

https://docs.rs/actix-http/2.0.0/src/actix_http/response.rs.html#603

However, it is possible to form a response with 0 copying using something like the following:

const body: Body = Body::Bytes(web::Bytes::from_static(
    include_bytes!("static/s.css")
));

...

    let resp: HttpResponse = HttpResponse::new(StatusCode::OK);
    let headers = resp.headers_mut();
    headers.append(key, value);

...

fn respond_to(req: &HttpRequest, item: Option<&Resource>) -> HttpResponse {
    if let Some((resp, etag, body)) = item {
        if !any_match(etag.as_ref(), req) {
            resp.status_mut() = StatusCode::PRECONDITION_FAILED;
            return resp;
        } else if !none_match(etag.as_ref(), req) {
            resp.status_mut() = StatusCode::NOT_MODIFIED;
            return resp;
        }

        resp.set_body(body)

    } else {
        HttpResponse::NotFound().body("Not found")
    }
}

I haven't implemented this yet so I'm not sure if it works, but if it does, it should be significantly faster than the current implementation.

kilork commented 4 years ago

This can actually work out. Go for it, I will add some performance tests soon, so we will be able to see this significiant difference.

You may actually test this even without changing a library, just make a quick 30 lines test app and check it with ab.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.