[Performance] repeated property spreading causes out of memory error #34599

Activity

1. cjc343 commented on Oct 22, 2019

   

   cjc343

   on Oct 22, 2019

   More actions

   Sample react app: https://github.com/cjc343/spread-operator-overflow

   Overflows with 21 for me
2. Ryan Cavanaugh (RyanCavanaugh)

   added

   BugA bug in TypeScriptA bug in TypeScript

   on Oct 30, 2019
3. Ryan Cavanaugh (RyanCavanaugh)

   assigned

   Wesley Wigham (weswigham)

   on Oct 30, 2019
4. Ryan Cavanaugh (RyanCavanaugh)

   added this to the TypeScript 3.8.0 milestone on Oct 30, 2019
5. RyanCavanaugh commented on Oct 30, 2019

   

   Ryan Cavanaugh (RyanCavanaugh)

   on Oct 30, 2019

   Member

   More actions

   I'm not confident there's going to be a straightforward fix available for this...
6. weswigham commented on Oct 30, 2019

   

   Wesley Wigham (weswigham)

   on Oct 30, 2019

   Last edited by weswigham

   Member

   More actions

   Oh boy. I know what's going on and you're not going to like it. So, let's break it down. Start with this:

   function parseWithSpread(config: Record<string, number>) {
       return {
           ...config.a !== undefined && { a: config.a.toString() },
       }
   }

   over in the playground, that'll have a return type of {}, but in reality before subtype reduction (done during return type fetching) it has a type of {} | { a: string }. Alright, let's add another field:

   function parseWithSpread(config: Record<string, number>) {
       return {
           ...config.a !== undefined && { a: config.a.toString() },
           ...config.b !== undefined && { b: config.b.toString() },
       }
   }

   OK, so the first spread is a union of {} | { a: string }, the second spread is then {} | { b: string }, which we then combine with the first spread - this means for each element of the first union, we combine with each element of the second spread. So we end up with {} | { b: string } | { a: string } | { a: string; b: string }. Alright, neat. Now let's add a 3rd:

   function parseWithSpread(config: Record<string, number>) {
       return {
           ...config.a !== undefined && { a: config.a.toString() },
           ...config.b !== undefined && { b: config.b.toString() },
           ...config.c !== undefined && { c: config.c.toString() },
       }
   }

   so let's spread in {} | { c: string }.... we get {} | { b: string } | { a: string } | { a: string; b: string } | {c: string} | { b: string; c: string; } | { a: string; c: string; } | { a: string; b: string; c: string }. Ah, hopefully by this point, the pattern is apparent - since a union representation is in use, each spread doubles the number of union elements under consideration. 2^25 is something like 33 million, so, naturally, we OOM at that point. The "fast" method is fast because it's less precise - at no point is a union of alternatives ever under consideration, really.

   😄React with 😄1Reacted by AnyhowStep
7. niedzielski commented on Oct 31, 2019

   

   stephen (niedzielski)

   on Oct 31, 2019

   Author

   More actions

   Thank you each for your considerations.

   Given the conclusion that the spreads are modeled as:

   type Spread =
     | {}
     | { a: string }
     | { b: string }
     | { c: string }
     | { a: string, b: string }
     | { b: string, c: string }
     | { a: string, c: string }
     | { a: string, b: string, c: string };
     // | ...

   Is there a more general (and hopefully performant) representation that could be used instead? I think this is saying "it is either present or missing but not undefined" so maybe #13195?
8. Wesley Wigham (weswigham)

   closed this as completedin #34853on Nov 23, 2019
9. mieszko4 commented on Jan 16, 2020

   

   mieszko4

   on Jan 16, 2020

   More actions

   I still have an issue with this on 3.8.0-beta so fix in #34853 did not solve the problem for me.

   Consider the following file test.js

   const filters = {
     f01: true,
   };
   const extendedFilter = {
     ...(filters.f01 ? { b01: 1 } : { a01: 2 }),
     ...(filters.f02 ? { b02: 1 } : { a02: 2 }),
     ...(filters.f03 ? { b03: 1 } : { a03: 2 }),
     ...(filters.f04 ? { b04: 1 } : { a04: 2 }),
     ...(filters.f05 ? { b05: 1 } : { a05: 2 }),
     ...(filters.f06 ? { b06: 1 } : { a06: 2 }),
     ...(filters.f07 ? { b07: 1 } : { a07: 2 }),
     ...(filters.f08 ? { b08: 1 } : { a08: 2 }),
     ...(filters.f09 ? { b09: 1 } : { a09: 2 }),
     ...(filters.f10 ? { b10: 1 } : { a10: 2 }),
     ...(filters.f11 ? { b11: 1 } : { a11: 2 }),
     ...(filters.f12 ? { b12: 1 } : { a12: 2 }),
     ...(filters.f12 ? { b12: 1 } : { a12: 2 }),
     ...(filters.f13 ? { b13: 1 } : { a13: 2 }),
     ...(filters.f14 ? { b14: 1 } : { a14: 2 }),
     ...(filters.f15 ? { b15: 1 } : { a15: 2 }),
     ...(filters.f16 ? { b16: 1 } : { a16: 2 }),
     ...(filters.f17 ? { b17: 1 } : { a17: 2 }),
     ...(filters.f18 ? { b18: 1 } : { a18: 2 }),
     ...(filters.f19 ? { b19: 1 } : { a19: 2 }),
     ...(filters.f20 ? { b20: 1 } : { a20: 2 }),
   };

   I get out of memory error when running ts test.js --allowJs

   Any suggestion how I could overcome this problem?
10. weswigham commented on Jan 16, 2020

    

    Wesley Wigham (weswigham)

    on Jan 16, 2020

    Member

    More actions

    We very narrowly optimized the repeaded spread of undefined | {p1: T} case; a repeated spread of {p1: T} | {h1: U} creates a exponential expansion in the number of types required to compute the expression. In effect, each line you've written there doubles the number of possible output types, and 2^20 is not a small number, by any means.
11. AnyhowStep commented on Jan 17, 2020

    

    AnyhowStep

    on Jan 17, 2020

    Contributor

    More actions

    2^20 is tiny compared to Tree(3), though

    Runs away

    😄React with 😄2Reacted by Joris W and conard-lamp
12. mieszko4 commented on Jan 17, 2020

    

    mieszko4

    on Jan 17, 2020

    More actions

    Wesley Wigham (@weswigham) thank you for the explanation.

    I've noticed that for the example I gave if I replace ... with Object.assign typescript works well.
    So maybe that could be a hint how the case with ... could be fixed in the core.

    🚀React with 🚀1Reacted by Limbo Sounds
13. jorisw commented on Aug 31, 2020

    

    Joris W (jorisw)

    on Aug 31, 2020

    Last edited by jorisw

    More actions

    I still encounter the memory leak with TypeScript 3.9.6 when using the spread operator to compose ThemeUI styles:

      const selectStyles: SxStyleProp = {
        ...defaultSelectStyles,
        ...filledOrEmptySelectStyles,
        ...(showSmallLabel ? withSmallLabelSelectStyles : {}),
        ...(focused || hovered ? focusedOrHoveredSelectStyles : {}),
        ...(readOnly || disabled ? disabledStyles : {}),
        ...(hovered ? hoveredStyles : {}),
        ...(focused ? focusedStyles : {}),
        ...(hasErrors ? errorStyles : {}),
        ...(hasWarnings ? warningStyles : {}),
      }

    Each of the objects being spread above are also typed as SxStyleProps.
14. dotansimha commented on Aug 31, 2020

    

    Dotan Simha (dotansimha)

    on Aug 31, 2020

    More actions

    Wesley Wigham (@weswigham) I just encountered this with TS 4.0.2. Has a file with ~50 spreads (🤦 ).

    If it's done this way:

    // something = any
    // somethingElse = any
    return {
      ...(something && { foo: 'foo' }),
      ...(somethingElse && { bar: 'bar' }),
    }

    If I set something or somethingElse to it's real value (boolean) it fails with an invalid memory allocation in V8 (running with 12GB of RAM...).

    Also, if I change this to:

    // something = any
    // somethingElse = any
    return {
      ...(something ? { foo: 'foo' } : {}),
      ...(somethingElse ? { bar: 'bar' } : {}),
    }

    It fails :(

    I guess the only solution here is to use Object.assign instead of spreads?

    👍React with 👍3Reacted by Josip Reh, Bibin Egor and Salem Raouf
15. microsoft

    locked as resolved and limited conversation to collaborators on Oct 21, 2025