Semantic-ui-react: perf(lodash): remove use of _.omit (and optimize lodash)

[levithomason]

_.omit

Per https://github.com/Semantic-Org/Semantic-UI-React/pull/860, we've improved the render times of all components by ~12,000 times with a 4 line vanilla _.omit replacement.

The issue here was that omit makes copies of its object arguments, the latter argument appears to be deep copied with circular reference checks. That second argument happened to be props, which of course includes children, which of course include children, ...omg.

Every component uses getUnhandledProps, on every render, so every update caused every component to make a deep copy of its props and children's props all the way down the render tree util it reached the end of the tree. Then, repeat this for every node in the tree. It is obvious why Grid and Grid column were top offenders, they are mostly likely to contain the most deeply nested children.

We have no need to ever make a full copy of objects like this. We ought to replace all uses of _.omit and other methods that copy objects and replace them with our own simple util.

Lodash must go :(

This would be a massive undertaking but we should, unfortunately, consider completely replacing lodash if we're serious about performance. This could be done with a shim module that replaces one method at a time (e.g. lib/lodash) and uses the lodash method if it has no replacement.

_What? Why would you?_

It's cherry-picked by babel-plugin-lodash, so it the footprint is small, right?

import omit from 'lodash/omit'

Webpack ^ this alone and you get this bundle:

• 96 modules
• 3,172 lines
• 83.6 kB

_All_ of this code is lodash. Its internal util dependency tree is _massive_, even when cherry-picking methods. This one method ends up taking what can be done in 4 LOC and turns it into an 84kb module with 96 dependencies. This is utterly insane.

_Isn't it really performant, sometimes faster than native?_

Sure, however, as shown above and in #860, we achieved >12,000 times performance gain with a 4 line vanilla JS method.

_How large is our cherry-picked lodash dependency?_

It's 30% of the uncompressed library size, 278kb. When Semantic-UI-React is minified and propTypes are striped, it is only 288K itself.

[jeffcarbs]

@levithomason - Two questions regarding performance:

• would you mind pasting the code you used for the getUnhandledProps performance tests or putting it into a gist? I've never really done performance testing in React so I'm interested to see what it entails.
• is it possibly to set these tests up in some automated way? It would be cool to have a standard set of performance benchmarks, perhaps using the examples from the doc site?

[levithomason]

I used a short hack:

import ReactPerf from 'react-addons-perf'
window.perf = ReactPerf

Then, on the console:

perf.start()
// do stuff you want to get perf on
perf.stop()
perf.printExclusive()

We could totally capture these results and do something automated I'm sure. Hadn't thought of that!

[markerikson]

I'll admit I'd be interested in hearing @jdalton 's thoughts on the perf and bundle size issues.

[jdalton]

Hi @markerikson!

Using webpack and babel plugins lodash/omit is 831 bytes _(< 1kB)_ minified+gzipped.

That said, _.omit is pretty slow compared to something like a _.pick for sure. To perform an omit we have to start with everything (all properties, own and inherited, string keys and symbols) and then omit a handful of properties. On the other hand, _.picking is pretty straight forward. So from a design standpoint I prefer that. Because of this we're dropping _.omit in v5.

From a perf standpoint Lodash is a collection of general purpose utilities. It's certainly the case that more specialized and context aware implementations can be faster. I'd say specialize where needed and defer to a utility lib in most other cases where you don't feel like the maintenance burden.

[markerikson]

Dropping omit? Yikes. I knew you had a bunch of changes planned, but that's one I use fairly frequently in my own apps. Somewhat off-topic, but what are you suggesting as a replacement for omit for most cases?

[jdalton]

@markerikson

Somewhat off-topic, but what are you suggesting as a replacement for omit for most cases?

From my initial reply:

On the other hand _.picking is pretty straight forward so from a design standpoint I prefer that.

It's easier to pick than to omit. A pick is also likely at least 4x faster than your current omit.

[levithomason]

@jdalton I very much appreciate the extra context and deeper insight. I would love to go with some solution similar to your last suggestion:

I'd say specialize where needed and defer to a utility lib in most other cases where you don't feel like the maintenance burden.

Any other info or resources you have regarding lodash perf would also be greatly appreciated.

[jdalton]

Any other info or resources you have regarding lodash perf would also be greatly appreciated.

I'm not familiar with your usage but if you give me a list of methods and scenarios I can review them.

[levithomason]

Looks like we're using 32 methods at present:

_.capitalize
_.clamp
_.compact
_.dropRight
_.each
_.escapeRegExp
_.every
_.filter
_.find
_.findIndex
_.get
_.has
_.head
_.inRange
_.includes
_.isEmpty
_.isEqual
_.isFunction
_.isNil
_.isUndefined
_.keys
_.map
_.omit
_.partialRight
_.reduce
_.round
_.sample
_.snakeCase
_.some
_.times
_.union
_.without

The usage scenarios are wide ranging but the majority of these methods are called at some point during the component render cycle. I don't think other usages would pose much of an issue since they are likely to be called very infrequently, or just once.

[jdalton]

@levithomason Cool.

With lodash-webpack-plugin the size is ~6kB min+gzip. Of those the most expensive are _.isEqual and _.partialRight. We're actually dropping the partial methods in v5 in favor of simply using arrow functions, so that's what I would recommend here. If you could just drop _.partialRight and _.isEqual that would land at ~4kb.

[levithomason]

This is awesome, we can certainly live with 4-6kB, even something many times this size is no issue IMO. I had not heard of the webpack plugin prior to your first response either. I'll drop that in and make the updates you've noted and see where we're at. Not sure when I'll get to this, but it won't be long. Thanks much!

[jdalton]

The _.isEqual function is heavy (size and perf) because it has to handle a lot of data types and situations (like circular references). I'd look to using a more specialized implementation for perf and size savings there.

[levithomason]

Yep, that one has been on the list to replace with shallowequal. I just checked and we're only using it in 3 modules, all of which can and _should_ be doing shallow comparisons anyway.

[jdalton]

Yep, that one has been on the list to replace with shallowequal.

I think even shallowequal may be too generic for your use.
A simple implementation on your end will end up with the best perf+size mix.

[levithomason]

Checking their source it does appear so. We only need a simple shallow strict equality check on object values. I'll likely implement this as well 👍