Make glamorous faster about glamorous

[developit]

You can skip an object copy by removing this line. Instead of "plucking" className off of props, simply use props.className. In order to have your custom value take precedence over props.className here just place the inline property after the rest spread.

Other things: remove map().

      render() {
        const props = this.props
        const {toForward, cssOverrides} = splitProps(props, GlamorousComponent)
        const theme = Object.assign({}, this.state.theme)
        // create className to apply
        const mappedArgs = GlamorousComponent.styles.slice();
        for (let i=mappedArgs.length; i--; ) {
          if (typeof mappedArgs[i]==='function') {
            mappedArgs[i] = mappedArgs[i](props, theme);
          }
        }
        const {
          glamorStyles: parentGlamorStyles,
          glamorlessClassName,
        } = extractGlamorStyles(props.className)
        const glamorClassName = css(
          ...mappedArgs,
          ...parentGlamorStyles,
          cssOverrides,
        ).toString()
        const fullClassName = joinClasses(glamorlessClassName, glamorClassName)

        return React.createElement(GlamorousComponent.comp, {
          ...toForward,
          className: fullClassName,
        })
      }

[kentcdodds]

I should also mention that I'm less interested in benchmarking against other solutions and more interested in benchmarking against ourselves so we can see our own improvement.

[kentcdodds]

I think that we could maybe write a babel plugin to take this:

const MyDiv = glamorous.div({padding: 20}, ({big}) => ({fontSize: big ? 20 : 30}))

to this:

const MyDiv = glamorous.div('css-dfk22d34', ({big}) => ({fontSize: big ? 20 : 30}))

Where the static styles are pre-processed and the resulting styles are extracted to a separate stylesheet, and we work with glamor's rehydration API to hook things up again. We'd need to update the glamorousComponentFactory API to accept a string class name, but that should be trivial. In any case, this would probably speed things up significantly.

I just realized that based on the way we merge glamor class names with the styles they represent into a single class name may cause issues with this, but let's start with components that have static styles only, then maybe we can get it working with a combination of static and dynamic styles.

Anyone wanna write a plugin for this? If you've not made a Babel plugin before or worked with ASTs before, you can learn about it from my talk :)

[kkemple]

i'll jump on it if no one else wants to do it, will let it sit for a day to see if anyone else wants to try. 😄

[tkh44]

@kentcdodds These numbers look grim until you realize that glamor is generating 1000 different classes. FWIW I've dug into the profiler and tried to pin down anything in glamorous that is the bottleneck and the biggest hot spots are in get-glamorous-classname.js. And the source of these hotspots is... glamor 😄

The only deopt I could find were in fast-memoize's variadic function. I'm guessing due to Array.prototype.slice.call(arguments, 3). If we know the arg length ahead of time why not write our own?

function variadic (fn, cache, serializer, arg1, arg2) {
  var args = [arg1, arg2]
  var cacheKey = serializer(args)

  if (!cache.has(cacheKey)) {
    var computedValue = fn.apply(this, args)
    cache.set(cacheKey, computedValue)
    return computedValue
  }

  return cache.get(cacheKey)
}

function strategy (fn, options) {
  return variadic.bind(
    this,
    fn,
    options.cache.create(),
    options.serializer
  )
}


memoize(shouldForwardProperty, { strategy })

Not even sure if it is worth the effort though.

[kentcdodds]

Not to stifle innovation! Happy to review PRs to make things faster so long as it doesn't make things too complicated :)

[kentcdodds]

Dude, you're the best, thanks for that @developit! Anyone wanna actually make these changes?

[kentcdodds]

Thanks @kkemple! I tweeted it out. We'll see if anyone else is interested in jumping on board.

[kkemple]

🎣

[garetmckinley]

I watched your talk about ASTs Kent. Supppppper informative! I've been wanting to dig into babel plugins for a little while. Was moderately disappointed when I realized frontend masters didn't have a course on ASTs or babel plugins, so your video was exactly the introduction I was looking for 💣 you have an awesome way of teaching that really makes information stick. Too many educators use a very "formal" way of teaching, but you walk us through figuring stuff out by console logging and using actual debugging tools.

That said, I want to take a whack at making a babel plugin for glamorous. If I read your message correctly, you're saying that it's not totally possible at the moment with glamorous due to the glamorousComponentFactory API, but that shouldn't stop me from being able to generate classes and mapping them to the component factory, right?

If somebody more experienced than me wants to create it, that's fine. But I'm going to treat creating a glamorous plugin as my introductory project to babel plugins. The only thing I'm unsure about is how to go about generating an eternal stylesheet to rehydrate glamorous with. I'll have to dig through the source a bit, it would be awesome if I could get pointed in the right direction 😄

edit: also I've been real busy with my day job this week, but I'll hopefully be able to get back to working on those benchmarks tonight. If not tonight, definitely this weekend.

[kentcdodds]

Was moderately disappointed when I realized frontend masters didn't have a course on ASTs or babel plugins

Fun fact, I'm giving an asts-workshop at Frontend Masters in 1.5 weeks!

I'm glad the video was helpful. And thanks so much for the feedback :)

If I read your message correctly, you're saying that it's not totally possible at the moment with glamorous due to the glamorousComponentFactory API, but that shouldn't stop me from being able to generate classes and mapping them to the component factory, right?

Correct. We can make the API changes separately. You can go ahead and start work on the babel plugin as soon as you want.

The only thing I'm unsure about is how to go about generating an eternal stylesheet to rehydrate glamorous with.

Yeah, this bit will be a little tricky, but it actually seems like you could take some inspiration from the plugin by @thejameskyle called babel-plugin-polished. Basically, when you find the right object literal, you require in glamor and run it on that object literal. The part where you'll divert from the polished plugin is that when the plugin's finished with a file, it'll need to save the CSS to another file somewhere. It may be a little tricky. Maybe @threepointone will have some tips...

I think this could be a really awesome plugin. Super cool experience for anyone wanting to get into this stuff.

also I've been real busy with my day job this week, but I'll hopefully be able to get back to working on those benchmarks tonight. If not tonight, definitely this weekend.

Hey man, don't overwork yourself. This is open source, if you can't do it, no biggie. Other people can make their own stuff if they really need to :) Thanks for any work you're able to do!

[edygar]

I've tried to add glamorous in this benchmark but something went really wrong, maybe just because I'm unexperienced. Have any of you tried?

[kentcdodds]

Hey @edygar! Thanks for trying that. I haven't tried that out yet. I've not really done much actual benchmarking yet :-/

[tkh44]

@kentcdodds I collected some rough data and put it into a spreadsheet here.

I used this little code snippet and checked dev tools profiles to get the data. I'm not sure how valuable this is but it's something 🙂

<!DOCTYPE html>
<html>
<head>
    <meta charset="utf-8"/>
    <title>Glamorous Render Test</title>
</head>
<body>
<div id="target"></div>
<script src="../node_modules/react/dist/react.js"></script>
<script src="../node_modules/react-dom/dist/react-dom.js"></script>
<script src="../node_modules/glamor/umd/index.min.js"></script>
<script src="../dist/glamorous.umd.js"></script>
<script type="text/javascript">
    'use strict';
  const Test = glamorous.div(({ i }) => ({
    background: '#f06595',
    borderBottom: '1px solid #a61e4d',
    height: 100,
    order: i,
    width: 100,
  }));

//  const Test = ({ i }) => {
//    return React.createElement(
//      'div',
//      {
//        style: {
//          background: '#f06595',
//          borderBottom: '1px solid #a61e4d',
//          height: 100,
//          order: i,
//          width: 100,
//        }
//      }
//    );
//  }

  const target = document.getElementById('target')
  console.profile();
  ReactDOM.render(
    React.createElement(
      glamorous.Div,
      {
//        css: {
//          '& > div': { marginBottom: 4 },
//          display: 'flex',
//          flexDirection: 'column',
//          alignItems: 'center'
//        }
      },
      Array.from({ length: 1000 }, (_, i) => React.createElement(Test, { i: i, key: i}))
    ),
    target
  );
  console.profileEnd();
</script>
</body>
</html>

I captured the glamor createElement data with this code https://gist.github.com/tkh44/c22d78245a9d208eea3f121c78065736

[kentcdodds]

Awesome! I wonder whether there's any way we can automate the generation of this data so we could add a log in our Travis CI to indicate whether we're faster or slower than master. May be kinda hard to do well though :-/

[kentcdodds]

Thanks for digging into that even further! Honestly, I think that it may be even better for us to see big applications built with glamorous and see how it performs in those cases. It's harder, but those are the benchmarks that really matter.

Considering that this is kind of an ongoing effort and I don't see any more big wins we can make here, I'm going to go ahead and close this and say that we're as fast as we need to be <3

Thanks everyone!