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

Exponential Type Blowup when wrapping function #68508

Open
gakonst opened this issue Jan 24, 2020 · 3 comments
Open

Exponential Type Blowup when wrapping function #68508

gakonst opened this issue Jan 24, 2020 · 3 comments
Labels
I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@gakonst
Copy link

gakonst commented Jan 24, 2020

I am implementing a service architecture, where each service can be thought of as middleware. It makes sense to define complex services which need to manage their own complex state as their own structs and implement my Service trait for each one. However, for simple operations, e.g. logging or manipulating some argument in place, it makes more sense to have a wrapper service which receives the previous service's "request", does whatever it needs to do, and passes it on. The wrapper service also implements the Service trait.

The problem here is that there is an exponential type length complexity blowup when passing in the function.

Probably related: #54540, note how there's no closure in my example

MVCE below: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=17d88bb1e552b2cc285ced44777e726b

(complex services: BaseService / MyService, stateless services: logger, doubler)

// The objective is to be able to wrap existing services with simple functions, without
// having to define a new service each time we want to perform a very minimal action
// on the data flow. We could define a LoggerService and a DoublingService, but that
// is _a lot_ of boilerplate.
fn main() {
    let s = BaseService;
    // We can create as many new services as we want,
    // the type length increases linearly, as expected
    let s = MyService::new(s);
    let s = MyService::new(s);
    // The type length of `s` doubles
    let s = WrappedService::new(s, stateless_log);
    // if you try to wrap a few times, your code hits the type length limit
    let s = WrappedService::new(s, stateless_double);
    let s = WrappedService::new(s, stateless_double);
    let s = WrappedService::new(s, stateless_double);
    let s = WrappedService::new(s, stateless_double);
    let s = WrappedService::new(s, stateless_double);
    let s = WrappedService::new(s, stateless_double);
    let s = WrappedService::new(s, stateless_double);
    let s = WrappedService::new(s, stateless_double);
    let s = WrappedService::new(s, stateless_double)
}

type Request = u64;
type MyResult = Result<(), ()>;

pub trait Service {
    fn handle(&mut self, request: Request) -> MyResult;
}

// A simple service which we we use as our base
#[derive(Clone)]
pub struct BaseService;
impl Service for BaseService {
    fn handle(&mut self, _request: Request) -> MyResult {
        Ok(())
    }
}

// A service that takes another service (and could have more state)
#[derive(Clone)]
pub struct MyService<O> {
    next: O,
}
impl<O> MyService<O>
where
    O: Service + Clone + Send,
{
    pub fn new(next: O) -> Self {
        MyService { next }
    }
}
impl<O> Service for MyService<O>
where
    O: Service + Clone + Send,
{
    fn handle(&mut self, request: Request) -> MyResult {
        self.next.handle(request)
    }
}

/// =========== Stateless Services =========

use std::sync::Arc;

// We have to store the service onion so far, and the function we want to inject
#[derive(Clone)]
pub struct WrappedService<F, I> {
    f: F,
    inner: Arc<I>,
}
impl<F, S> WrappedService<F, S>
where
    F: Fn(S, Request) -> MyResult,
    S: Service + Clone,
{
    pub fn new(inner: S, f: F) -> Self {
        WrappedService {
            f,
            inner: Arc::new(inner),
        }
    }
}
impl<F, S> Service for WrappedService<F, S>
where
    F: Fn(S, Request) -> MyResult,
    S: Service + Clone,
{
    fn handle(&mut self, request: Request) -> MyResult {
        (self.f)((*self.inner).clone(), request)
    }
}

// Is there a way to convert these to a service, without having to go through a lot of boilerplate?
pub fn stateless_log<O: Service + Clone>(mut next: O, request: Request) -> MyResult {
    println!("sending request");
    let result = next.handle(request);
    println!("got result");
    result
}

pub fn stateless_double<O: Service + Clone>(mut next: O, request: Request) -> MyResult {
    let request = 2 * request;
    let result = next.handle(request);
    result
}

(Playground)

@jonas-schievink jonas-schievink added I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 24, 2020
@gakonst
Copy link
Author

gakonst commented Jan 29, 2020

Converting the function's argument to a Boxed trait object as a workaround did the trick: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=68c09468854a00faa2b5b9fb000e578b

75,76c75,76
<     F: Fn(S, Request) -> MyResult,
<     S: Service + Clone,
---
>     F: Fn(Box<dyn Service>, Request) -> MyResult,2
>     S: Service + Clone + 'static,
87,88c87,88
<     F: Fn(S, Request) -> MyResult,
<     S: Service + Clone,
---
>     F: Fn(Box<dyn Service>, Request) -> MyResult,
>     S: Service + Clone + 'static,
91c91
<         (self.f)((*self.inner).clone(), request)
---
>         (self.f)(Box::new((*self.inner).clone()), request)
96c96
< pub fn stateless_log<O: Service + Clone>(mut next: O, request: Request) -> MyResult {
---
> pub fn stateless_log(mut next: Box<dyn Service>, request: Request) -> MyResult {
103c103
< pub fn stateless_double<O: Service + Clone>(mut next: O, request: Request) -> MyResult {
---
> pub fn stateless_double(mut next: Box<dyn Service>, request: Request) -> MyResult {

@bstrie
Copy link
Contributor

bstrie commented Jan 29, 2020

Last year we had shown the aforementioned service architecture and its compilation-time woes to @nikomatsakis , who expressed some interest in hearing about whether we could manage to find a solution. (Though he may have been more interested in the fact that we were needing to introduce ever-escalating #![type_length_limit = "5000000"] attributes in order to even compile at all. :P ) We had previously tried boxing plenty of things in a relatively haphazard way without much luck at addressing or discovering the underlying problem. However, with the boxing strategy shown here we seem to have cut our typical compilation times in half (though we have not yet successfully rid ourselves of the type_length_limit attributes, whose values still somehow seem to keep rising...).

gakonst added a commit to interledger/interledger-rs that referenced this issue Jan 29, 2020
* feat(service): upgrade to futures 0.3 and async/await

* feat(service): Box wrapper methods to avoid exponential type blowup

Relevant rust-lang issue: rust-lang/rust#68508

* docs(service): add explanation on IlpResult

* chore(service): remove unused associated type

* # Interledger Router: Futures 0.3 Transition (#595)

* feat(router): upgrade to futures 0.3 and async/await

# Interledger ILDCP: Futures 0.3 Transition (#597)

* feat(client): convert client to async/await

* docs(ildcp): enhance docs

* feat(server): make the service async

* test(server): add tests

# Interledger CCP: Futures 0.3 Transition (#598)

* feat(ccp): convert store traits to async/await

* feat(ccp-server): make the ccp server async

* test(ccp-server): make tests async

* chore(routing-table): limit api visibility of table methods

# Interledger BTP: Futures 0.3 Transition  (#599)

* feat(btp): update traits to be async

* refactor(btp/wrapped-ws): refactor WsWrap to a separate file

Ideally, we would want to get rid of it by doing a `StreamExt::map_ok` and `SinkExt::with` to map both WebSocket return types to the same value. We also use `filter_map` to get rid of any errors from the WebSocket. The WsError type has been removed as a result of that.

* feat(btp/client): port to async/await

* feat(btp/server): move to async/await

* feat(btp/service): move service to async/await

* We refactored the service to be more readable. Basically, we split the websocket in a Sink (write) and a Stream (read). We also create a `tx`/`rx` pair per account. The rx receiver gets attached to the sink, meaning any data sent over by the `tx` sender will get forwarded to the sink, which will forward it to the other end of the websocket. Unfortunately, due to being unable to combine the read and write sockets, we have to spawn them separately. This means that we have to remove the hook which cancels the streams.

# Interledger HTTP: Futures 0.3 Transition  (#600)

* feat(http): Update HttpStore trait to futures 0.3 and deserialize_json method

* feat(http): Update HTTP Errors and client

* feat(http): Update HTTP Server

* docs(http): extend http docs

# Interledger Stream: Futures 0.3 Transition  (#601)

* feat(stream): Update Stream server

* feat(stream): Update Stream client

* docs(stream): extend stream docs

* fix(stream): add extra limits to ensure all the pending request futures are thread safe

# Interledger Settlement: Futures 0.3 Transition  (#602)

* feat(settlement/core): Upgrade types and idempotency

* feat(settlement/core): Upgrade engines API Warp interface

* feat(settlement/core): Upgrade Redis backend implementation

* feat(settlement/api): Upgrade the message service

* feat(settlement/api): Upgrade the settlement client

* feat(settlement/api): Upgrade the Settlement API exposed by the node

* chore(settlement): remove need to pass future wrapped in closure

* docs(settlement): extend settlement docs

# Interledger SPSP: Futures 0.3 Transition (#603)

* feat(spsp): move to futures 0.3 and async/await

* docs(spsp): extend spsp docs

* fix(spsp): tighten trait bounds to account for stream changes

# Interledger Service Util: Futures 0.3 Transition  (#604)

* feat(service-util): update validator service

* feat(service-util): update rate limit service

* feat(service-util): update max packet amount service

* feat(service-util): update expiry shortener service

* feat(service-util): update exchange rate service and providers

* feat(service-util): update echo service

* feat(service-util): update balance service

# Interledger API: Futures 0.3 Transition  (#605)

* feat(api): update trait definitions and dependencies

* feat(api): update http retry client

* test(api): migrate test helpers

* feat(api): update node-settings route

* test(api): update node-settings route tests

* feat(api): update accounts route

* test(api): update accounts route tests

* chore(api): add missing doc

# Interledger Store: Futures 0.3 Transition (#606)

* feat(store): Update redis reconnect

* feat(store): Update base redis struct

* feat(store): Update AccountStore trait

* feat(store): Update StreamNotificationsStore trait

* feat(store): Update BalanceStore trait

* feat(store): Update BtpStore trait

* feat(store): Update HttpStore trait

* feat(store): Update NodeStore trait

* feat(store): Update AddressStore trait

* feat(store): Update RouteManagerStore trait

* feat(store): Update RateLimitStore trait

* feat(store): Update IdempotentStore trait

* feat(store): Update SettlementStore trait

* feat(store): Update LeftoversStore trait

* feat(store): Update update_routes

* test(store): convert all tests to tokio::test with async/await

* feat(store): update secrecy/bytes/zeroize

* docs(store): add more docs

# ILP CLI: Futures 0.3 Transition (#607)

* feat(ilp-cli): update CLI to async/await

# ILP Node: Futures 0.3 Transition (#608) (#609)

* test(ilp-node): migrate tests to futures 0.3

* feat(ilp-node): move metrics related files to feature-gated module

* feat(ilp-node): remove deprecated insert account function

* feat(ilp-node): make the node run on async/await

* ci(ilp-node): disable some advisories and update README

* fix(ilp-node): spawn prometheus filter
gakonst added a commit to interledger/interledger-rs that referenced this issue Jan 29, 2020
* feat(packet): implement From<Fulfill/Reject> for bytes05::BytesMut

ILP-Packet is built using Bytes 0.4. The Futures 0.3 ecosystem's HTTP crates use Bytes 0.5.
Porting this crate to use Bytes 0.5 is non-trivial due to significant breaking changes in the
Bytes API:

tokio-rs/bytes#350
tokio-rs/bytes#288

# Interledger Service: Futures 0.3 Transition (#596)

* feat(service): upgrade to futures 0.3 and async/await

* feat(service): Box wrapper methods to avoid exponential type blowup

Relevant rust-lang issue: rust-lang/rust#68508

* docs(service): add explanation on IlpResult

* chore(service): remove unused associated type

# Interledger Router: Futures 0.3 Transition (#595)

* feat(router): upgrade to futures 0.3 and async/await

# Interledger ILDCP: Futures 0.3 Transition (#597)

* feat(client): convert client to async/await

* docs(ildcp): enhance docs

* feat(server): make the service async

* test(server): add tests

# Interledger CCP: Futures 0.3 Transition (#598)

* feat(ccp): convert store traits to async/await

* feat(ccp-server): make the ccp server async

* test(ccp-server): make tests async

* chore(routing-table): limit api visibility of table methods

# Interledger BTP: Futures 0.3 Transition  (#599)

* feat(btp): update traits to be async

* refactor(btp/wrapped-ws): refactor WsWrap to a separate file

Ideally, we would want to get rid of it by doing a `StreamExt::map_ok` and `SinkExt::with` to map both WebSocket return types to the same value. We also use `filter_map` to get rid of any errors from the WebSocket. The WsError type has been removed as a result of that.

* feat(btp/client): port to async/await

* feat(btp/server): move to async/await

* feat(btp/service): move service to async/await

* We refactored the service to be more readable. Basically, we split the websocket in a Sink (write) and a Stream (read). We also create a `tx`/`rx` pair per account. The rx receiver gets attached to the sink, meaning any data sent over by the `tx` sender will get forwarded to the sink, which will forward it to the other end of the websocket. Unfortunately, due to being unable to combine the read and write sockets, we have to spawn them separately. This means that we have to remove the hook which cancels the streams.

# Interledger HTTP: Futures 0.3 Transition  (#600)

* feat(http): Update HttpStore trait to futures 0.3 and deserialize_json method

* feat(http): Update HTTP Errors and client

* feat(http): Update HTTP Server

* docs(http): extend http docs

# Interledger Stream: Futures 0.3 Transition  (#601)

* feat(stream): Update Stream server

* feat(stream): Update Stream client

* docs(stream): extend stream docs

* fix(stream): add extra limits to ensure all the pending request futures are thread safe

# Interledger Settlement: Futures 0.3 Transition  (#602)

* feat(settlement/core): Upgrade types and idempotency

* feat(settlement/core): Upgrade engines API Warp interface

* feat(settlement/core): Upgrade Redis backend implementation

* feat(settlement/api): Upgrade the message service

* feat(settlement/api): Upgrade the settlement client

* feat(settlement/api): Upgrade the Settlement API exposed by the node

* chore(settlement): remove need to pass future wrapped in closure

* docs(settlement): extend settlement docs

# Interledger SPSP: Futures 0.3 Transition (#603)

* feat(spsp): move to futures 0.3 and async/await

* docs(spsp): extend spsp docs

* fix(spsp): tighten trait bounds to account for stream changes

# Interledger Service Util: Futures 0.3 Transition  (#604)

* feat(service-util): update validator service

* feat(service-util): update rate limit service

* feat(service-util): update max packet amount service

* feat(service-util): update expiry shortener service

* feat(service-util): update exchange rate service and providers

* feat(service-util): update echo service

* feat(service-util): update balance service

# Interledger API: Futures 0.3 Transition  (#605)

* feat(api): update trait definitions and dependencies

* feat(api): update http retry client

* test(api): migrate test helpers

* feat(api): update node-settings route

* test(api): update node-settings route tests

* feat(api): update accounts route

* test(api): update accounts route tests

* chore(api): add missing doc

# Interledger Store: Futures 0.3 Transition (#606)

* feat(store): Update redis reconnect

* feat(store): Update base redis struct

* feat(store): Update AccountStore trait

* feat(store): Update StreamNotificationsStore trait

* feat(store): Update BalanceStore trait

* feat(store): Update BtpStore trait

* feat(store): Update HttpStore trait

* feat(store): Update NodeStore trait

* feat(store): Update AddressStore trait

* feat(store): Update RouteManagerStore trait

* feat(store): Update RateLimitStore trait

* feat(store): Update IdempotentStore trait

* feat(store): Update SettlementStore trait

* feat(store): Update LeftoversStore trait

* feat(store): Update update_routes

* test(store): convert all tests to tokio::test with async/await

* feat(store): update secrecy/bytes/zeroize

* docs(store): add more docs

# ILP CLI: Futures 0.3 Transition (#607)

* feat(ilp-cli): update CLI to async/await

# ILP Node: Futures 0.3 Transition (#608) (#609)

* test(ilp-node): migrate tests to futures 0.3

* feat(ilp-node): move metrics related files to feature-gated module

* feat(ilp-node): remove deprecated insert account function

* feat(ilp-node): make the node run on async/await

* ci(ilp-node): disable some advisories and update README

* fix(ilp-node): spawn prometheus filter
@davidpdrsn
Copy link

I’m seeing a similar issue when using Tower and impl Trait. Made issue on rust-analyzer but “cargo build” is also slow so probably also a rust issue rust-lang/rust-analyzer#7403

it includes a reproduction script.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants