Performance of useFlags hook severely degraded by Proxy implementation

[coreylight]

With the introduction of this change in 2.27.0

flags object (that is either injected via props using LDConsumer or returned from the useFlags hook) will generate a flag evaluation event on flag read (using a JavaScript proxy). This can be disabled by setting reactOptions.sendEventsOnFlagRead: false.

Our application was unknowingly heavily impacted at runtime.
We have hundreds of flags and have expected a "plain object" implementation found in previous versions that caused no issues.

Example code that was ok before:

const useMyFlag = (name) => {
  const allFlags = useFlags();
  const newFlagsWithOverrides = {
    ...allFlags,
    ...overrides
  }
  return newFlagsWithOverrides[name];
}

While this code may not apply to everyone, it's very simple JS code that no developer would expect to cause performance implications.

By introducing a Proxy that does work based on key access, performance plummeted for us.

I'm raising an issue mostly to warn other developers to make certain they know of this change. To that effect, a major version bump should (and still can) be considered to this library.
OR the flag evaluation proxy should be opt-in.

[yusinto]

Thanks for reporting this. Are you able to share the number of flags involved when you start noticing the performance degradation please? If possible also please provide some metrics of the performance degradation in terms of rendering time for example comparing the performance before and after this issue.

[coreylight]

@yusinto We had about 520+ flags at time of the issue.
I've created a sample application that shows off the issue.
You are welcome to replace the sleep function with any function that does anything. But you can see through traces that the work adds up quickly, making the UI very slow to render.

[tarqd]

Hey @coreylight,

I think that this implementation would improve performance and ensure that flag events work as expected:

const useMyFlag = (name, fallback) => {
  const ldClient = useLDClient();
  if (overrides.hasOwnProperty(name)) {
    return overrides[name]
  }
  return ldClient.variation(name, fallback)
}

This will avoid any deoptimization caused by the use of Proxy and superfluous object creation. My hunch is that a majority of the performance loss is from deoptimization rather than the call to variation itself.

Although I highly recommend using variation to ensure platform features that rely on events funciton correctly, you may be able to use allFlags as a workaround:

const useMyFlag = (name) => {
  const ldClient = useLDClient();
  // directly access the flag set without the proxy
  const allFlags = ldClient.allFlags()
  //..... etc
}

[coreylight]

@tarqd Thanks for the suggestions. However, you're asking me to change my access patterns, which I've already done since we've experienced the pain of this change.
If LD doesn't want to make this opt-in, so be it. I've stated the regression that was caused by our team, using the methods that are outlined in the React Getting Started docs. If ldClient.variation is the most important method to use, I'd suggest the LD docs get updated to reflect that. And potentially adding a useFlagVariation hook for ease (if not already available).

[coracuity]

We just got bit by this problem as well, gaining back 4 seconds of initial load time by using allFlags() -- this seems like a really bad issue to let sit. It gradually over time makes your pages slower as your app grows and you use LD more, but each step along the way isn't alarming enough of a change in performance to be noticeable.

[gethari]

ok so I get that from multiple long pending critical issues like this, this library is not something to be used ?