-
Notifications
You must be signed in to change notification settings - Fork 47.2k
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
Experiment with using an object literal for Fiber creation #28734
Conversation
834d891
to
16e5343
Compare
Comparing: e02baf6...f349060 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
Awesome! It definitely makes sense to proceed if we can confirm that this is a perf win on Hermes. It should be given our earlier profiling results but let's check first? |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
The RN code from prod looks like this: function FiberNode(tag, pendingProps, key, mode) {
this.tag = tag;
this.key = key;
this.sibling =
this.child =
this.return =
this.stateNode =
this.type =
this.elementType =
null;
this.index = 0;
this.refCleanup = this.ref = null;
this.pendingProps = pendingProps;
this.dependencies =
this.memoizedState =
this.updateQueue =
this.memoizedProps =
null;
this.mode = mode;
this.subtreeFlags = this.flags = 0;
this.deletions = null;
this.childLanes = this.lanes = 0;
this.alternate = null;
}
function createFiberImplClass(tag, pendingProps, key, mode) {
return new FiberNode(tag, pendingProps, key, mode);
}
function createFiberImplObject(tag, pendingProps, key, mode) {
return {
tag: tag,
key: key,
elementType: null,
type: null,
stateNode: null,
return: null,
child: null,
sibling: null,
index: 0,
ref: null,
refCleanup: null,
pendingProps: pendingProps,
memoizedProps: null,
updateQueue: null,
memoizedState: null,
dependencies: null,
mode: mode,
flags: 0,
subtreeFlags: 0,
deletions: null,
lanes: 0,
childLanes: 0,
alternate: null
};
}
var createFiber = enableObjectFiber
? createFiberImplObject
: createFiberImplClass; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I wonder if we should just unify it to be an object literal -- is the object literal slower on v8? I'm happy to profile this in v8 too
Object literals should be faster at least on React Native with Hermes as the JS engine. It might also be interesting to confirm the old comments in this file from years ago are even still valid. Creating an object from a literal should be a simpler operation. It's a bit unfortunate that this introduces a bunch of copied code, but since we rearely update the fields on fibers, this seems like an okay tradeoff for a hot code path. An alternative would be some sort of macro system, but that doesn't seem worth the extra complexity.
Object literals should be faster at least on React Native with Hermes as the JS engine. It might also be interesting to confirm the old comments in this file from years ago are even still valid. Creating an object from a literal should be a simpler operation. It's a bit unfortunate that this introduces a bunch of copied code, but since we rearely update the fields on fibers, this seems like an okay tradeoff for a hot code path. An alternative would be some sort of macro system, but that doesn't seem worth the extra complexity. DiffTrain build for [fe98289](fe98289)
Object literals should be faster at least on React Native with Hermes as the JS engine. It might also be interesting to confirm the old comments in this file from years ago are even still valid. Creating an object from a literal should be a simpler operation. It's a bit unfortunate that this introduces a bunch of copied code, but since we rearely update the fields on fibers, this seems like an okay tradeoff for a hot code path. An alternative would be some sort of macro system, but that doesn't seem worth the extra complexity. DiffTrain build for commit fe98289.
Object literals should be faster at least on React Native with Hermes as the JS engine.
It might also be interesting to confirm the old comments in this file from years ago are even still valid. Creating an object from a literal should be a simpler operation.
It's a bit unfortunate that this introduces a bunch of copied code, but since we rearely update the fields on fibers, this seems like an okay tradeoff for a hot code path. An alternative would be some sort of macro system, but that doesn't seem worth the extra complexity.