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 blowup in core::drop_in_place symbol size for builder-pattern structures that chain closures #109363

Closed
sapphire-arches opened this issue Mar 19, 2023 · 3 comments
Labels
C-bug Category: This is a bug. 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

@sapphire-arches
Copy link
Contributor

I tried this code:

use std::marker::PhantomData;

struct NonTrivialDrop {}

impl Drop for NonTrivialDrop {
    fn drop(&mut self) {
    }
}

struct Builder<'a, F: FnOnce() -> String + 'a>(F, PhantomData<&'a ()>, NonTrivialDrop);

impl<'a, F: FnOnce() -> String + 'a> Builder<'a, F> {
    fn new(f: F) -> Self {
        Self(f, PhantomData, NonTrivialDrop {})
    }

    fn map(
        self,
        f: impl FnOnce(&String) + 'a,
    ) -> Builder<'a, impl FnOnce() -> String + 'a> {
        Builder::new(move || {
            let el = (self.0)();
            f(&el);
            el
        })
    }

    fn attr(
        self,
        name: &'a str,
        value: impl AsRef<str> + 'a,
    ) -> Builder<'a, impl FnOnce() -> String + 'a> {
        self.map(move |el| {
            let mut el = el.clone();
            el.push_str(name);
            el.push_str(value.as_ref());
        })
    }
}

fn main() {
    let mut b = Builder::new(|| String::new())
        .attr("a-0", "b")
        .attr("a-1", "b")
        .attr("a-2", "b")
        .attr("a-3", "b")
        .attr("a-4", "b")
        .attr("a-5", "b")
        .attr("a-6", "b")
        .attr("a-7", "b")
        .attr("a-9", "b");

    unsafe {
        // XXX: this is clearly UB, but the point is to generate a huge symbol name, not to write a
        // working program
        core::ptr::drop_in_place(&mut b);
    }
}

I expected to see this happen: the compiler will complete in a reasonable amount of time, and generate a suitably small symbol name.

Instead, this happened: The compile time of this program grows roughly proportional to 2 ** x where x is the number of .attr calls chained on the builder. The root cause of this seems to be exponential blowup in the size of the Builder type fed in to core::ptr::drop_in_place. With 16 chained .attr calls, the symbol name generated for core::ptr::drop_in_place is over 11 megabytes in size:

 objdump -x target/debug/bad | wc -L
11141131

This is probably related to #68508.

Meta

rustc --version --verbose:

rustc 1.70.0-nightly (13afbdaa0 2023-03-17)
binary: rustc
commit-hash: 13afbdaa0655dda23d7129e59ac48f1ec88b2084
commit-date: 2023-03-17
host: x86_64-unknown-linux-gnu
release: 1.70.0-nightly
LLVM version: 15.0.7
@sapphire-arches sapphire-arches added the C-bug Category: This is a bug. label Mar 19, 2023
@sapphire-arches sapphire-arches changed the title Exponential blowup in core::drop_in_place sybmol size for builder-pattern structures that chain closures Exponential blowup in core::drop_in_place symbol size for builder-pattern structures that chain closures Mar 20, 2023
@sapphire-arches
Copy link
Contributor Author

Thinking about this some more, the exponential size makes sense -- the type of each successive Builder::map closure includes the type of the previous closure twice (once for the ::attr calls and once for the ::map call). It's still unfortunate that the huge type bleeds in to the mangled name though, as it can cause problems like the one in the bindgen issue linked above. I'm sure native code linkers don't like a 100kb symbol name any better either.

@sapphire-arches
Copy link
Contributor Author

This is fixed by enabling v0 name mangling (#60705)

@jyn514 jyn514 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 Mar 25, 2023
@jyn514
Copy link
Member

jyn514 commented Mar 25, 2023

I'm going to close this in favor of #60705.

@jyn514 jyn514 closed this as not planned Won't fix, can't repro, duplicate, stale Mar 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. 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

2 participants