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

AddAnyAttr on AnyView: Large Compile Times, Overflows, & Crashes #3476

Open
geoffreygarrett opened this issue Jan 10, 2025 · 8 comments
Open

Comments

@geoffreygarrett
Copy link
Contributor

Below is a concise timeline of attempts and discussions related to enabling AddAnyAttr on AnyView. These efforts have repeatedly run into problems like extremely large compile times, linker or LLVM errors (e.g. “cannot encode offset of relocations; object file too large”), and recursion issues. All references below are from the leptos-rs/leptos repository.


Timeline

  1. Compile-Time Erasure Experiments

  2. Discovery of AddAnyAttr Incompatibilities

    • Issue Inconsistencies with --cfg=erase_components #3156
      • Reported that with --cfg=erase_components turned on, attr: was ignored in some components. Also noted that no working implementation for AddAnyAttr on AnyView exists without causing crashes or linker errors.
  3. Latest Attempts at AnyView + AddAnyAttr

In all these PRs, @zakstucke consistently reports the crash on larger codebases (macOS toolchain often yields cannot encode offset of relocations; object file too large). Removing or neutering the AnyView + AddAnyAttr relationship immediately fixes these crashes.


Potential Directions

  • Feature-Gating
    Temporarily hide or no-op AddAnyAttr for AnyView behind a Cargo feature or cfg until a stable fix is found.

  • Alternative Implementations
    Investigate deeper trait structures, or reduce recursion/boxing within AnyView. Some attempts hoisted dyn trait objects for attribute application but still triggered compiler blow-ups.

  • Compiler/LLVM Investigation
    The final crash is an LLVM Mach-O back-end limitation (see MachObjectWriter.cpp:925-931 for the relevant lines). Possibly open an LLVM bug or check if we can reduce the code path that leads to “object file too large.”

If anyone has fresh ideas or wants to collaborate on a fix, please chime in here!

@zakstucke @gbj @geoffreygarrett @sabify

@geoffreygarrett
Copy link
Contributor Author

Using #3460, I'm thinking either opt-in or opt-out is fine, but I'd like to rely on this for now. Here's the gating snippet:

#[cfg(not(feature = "avoid_anyview_attr"))]
impl AddAnyAttr for AnyView {
    type Output<SomeNewAttr: Attribute> = Self;

    fn add_any_attr<NewAttr: Attribute>(
        self,
        attr: NewAttr,
    ) -> Self::Output<NewAttr>
    where
        Self::Output<NewAttr>: RenderHtml,
    {
        let attr = attr.into_cloneable_owned();
        self.value.dyn_add_any_attr(attr.into_any_attr())
    }
}

#[cfg(feature = "avoid_anyview_attr")]
impl AddAnyAttr for AnyView {
    type Output<SomeNewAttr: Attribute> = Self;

    fn add_any_attr<NewAttr: Attribute>(
        self,
        _attr: NewAttr,
    ) -> Self::Output<NewAttr>
    where
        Self::Output<NewAttr>: RenderHtml,
    {
        // In "avoid_anyview_attr" mode, skip attribute merging:
        self
    }
}

Let me know if there's a preferred default (opt-in vs. opt-out). This at least makes it easier to move forward in the meantime.

@94bryanr
Copy link

Until a fix is available, I just want to comment some of the workarounds I have found reading other related issues with compiling after upgrading leptos from 0.6 to 0.7.

Workaround 1:

rustflags = [
 "-C", "link-args=-fuse-ld=lld"
]

Using lld to link instead of ld helps get a project to build on a mac.
On linux you will still get out of memory errors.

Workaround 2:

rustflags = [
 "-Csplit-debuginfo=unpacked"
]

Apparently this was a solution for this issue. This did not work for me.

Workaround 3:

rustflags = [
 "--cfg=erase_components"
]

Offered as a solution in #3433.
No change after enabling this for me.

Workaround 4:
Sprinkle .into_any() on a majority of your component definitions.
This fixes errors about types having too many levels of recursion (#![recursion_limit = "256"] error), but does not fix out of memory issues.


It looks like a bunch of the compile time recursive type issues were closed and now point to rust-lang/rust#130729. Though this is definitely a problem that leptos needs to handle - as going from 0.6 to 0.7 has, for my project:

  • caused the ld compiler to stop working on mac
  • created these out of memory issues on linux (CI pipeline)
  • increased development build times from ~5 seconds per change to now taking ~20 seconds

I still have not been able to get my project to reliably build after upgrading. If these issues are all due to this AddAnyAttr on AnyView impl then I do think this should be feature gated to be opt out by default until a more reliable solution can be found. Out of memory and compiler crashes are not issues you expect to run into when upgrading to the next release version and they are relatively painful to fix/diagnose given the different errors between platforms / compilers / environment.

@gbj
Copy link
Collaborator

gbj commented Jan 14, 2025

If these issues are all due to this AddAnyAttr on AnyView impl

@94bryanr While these workarounds are all helpful, I don't think that any of the issues you list are related to the topic, as the AddAnyAttr implementation for AnyView is not included on main or on any of the released 0.7 versions, specifically because of the linker issues it generated.

@gbj
Copy link
Collaborator

gbj commented Jan 14, 2025

Ultimately to my mind the broader compiler and linker problems are basically a Rust compiler issue: Unfortunately the language allows you to express things that the compiler is not able to handle at larger scales.

This was not discovered until I had put a huge amount of effort into the 0.7 work. It's possible this was just a bad decision, and that work was ultimately the wrong direction due to the compiler problems it creates on larger projects. It's possible that the framework should be rewritten again to something more like the previous version, despite the runtime downsides. If that's the case, it is work that someone else will probably have to take on.

@gbj
Copy link
Collaborator

gbj commented Jan 14, 2025

Oh and one last note for you @94bryanr on the incremental build times in particular — splitting your application into multiple crates may be the best advice here. Rust compiles with the crate as the basic compilation unit, which means for each incremental change (say, to one text node of one component of one page of your application) it will need to recompile the whole application crate. This has always been true, but is probably exacerbated by the more-heavily-generic rendering approach of 0.7. Breaking into multiple crates may be able to reduce the incremental build times again significantly.

@94bryanr
Copy link

@gbj I apologize for going off-topic. I misunderstood the title of this bug as being a sort of "catch-all" for the compiler/linker issues. Regardless, I was able to stabilize my CI build by using into_any() more liberally.

Leptos is pushing the boundaries of what is possible with Rust, and knowing beforehand how a change like this will affect every downstream project is impossible. The work you and the other contributors have done is greatly appreciated. I do not think reverting to what 0.6 was doing is the answer for larger projects, but it would be useful to keep an issue open on reducing the compile times for the new generic-based type system; as there may be ways to do this. Please let me know if it is reasonable to open another issue or if I should reach out on Discord.

Please feel free to mark these comments as off-topic as well.

@gbj
Copy link
Collaborator

gbj commented Jan 15, 2025

Oh, glad you were able to stabilize the build! And no worries at all. It will be really helpful to point people to the recommendations in the comment. I didn't mean to jump on you, so sorry about that -- I have genuinely gotten a bit discouraged about the bizarre issues this approach keeps turning up, and the overall compile-time issues are troubling too 😅 Thanks for your kind words.

EDIT: Oh and to your question; I think opening a new issue to track compile-time issues and suggestions would be great.

@geoffreygarrett
Copy link
Contributor Author

geoffreygarrett commented Jan 17, 2025

@zakstucke I reset #3461 onto yours and added a few more optimizations with #0db3233. Could you let me know if there is any improvement, even if just compile time?

The most noteworthy changes are:

DynValue

 impl<T> DynValue for T
 where
     T: Send,
     T: RenderHtml + 'static,
     T::State: 'static,
 {
+    #[inline(never)]
     fn dyn_add_any_attr(self: Box<Self>, attr: AnyAttribute) -> AnyView {
-        self.add_any_attr(attr).into_any()
+        DynValueAttr::apply_attr(self, attr)
     }
 
     fn dyn_any(self: Box<Self>) -> Box<dyn Any + Send> {
@@ -348,9 +364,11 @@ impl Render for AnyView {
     }
 }

AddAnyAttr

+// Pre-erases output to reduce compile-time explosions
 impl AddAnyAttr for AnyView {
-    type Output<SomeNewAttr: Attribute> = Self;
+    type Output<SomeNewAttr: Attribute> = AnyView;
 
+    #[inline(never)]
     fn add_any_attr<NewAttr: Attribute>(
         self,
         attr: NewAttr,
@@ -359,7 +377,8 @@ impl AddAnyAttr for AnyView {
         Self::Output<NewAttr>: RenderHtml,
     {
         let attr = attr.into_cloneable_owned();
         self.value.dyn_add_any_attr(attr.into_any_attr())
     }
 }

EDIT: Missing punctuation mark

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

No branches or pull requests

3 participants