Skip to content
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

Remove event pooling in the modern system #18969

Merged
merged 1 commit into from
May 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
163 changes: 92 additions & 71 deletions packages/legacy-events/SyntheticEvent.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,73 +157,79 @@ Object.assign(SyntheticEvent.prototype, {
* won't be added back into the pool.
*/
persist: function() {
this.isPersistent = functionThatReturnsTrue;
// Modern event system doesn't use pooling.
if (!enableModernEventSystem) {
this.isPersistent = functionThatReturnsTrue;
}
},

/**
* Checks if this event should be released back into the pool.
*
* @return {boolean} True if this should not be released, false otherwise.
*/
isPersistent: functionThatReturnsFalse,
isPersistent: enableModernEventSystem
? functionThatReturnsTrue
: functionThatReturnsFalse,

/**
* `PooledClass` looks for `destructor` on each instance it releases.
*/
destructor: function() {
const Interface = this.constructor.Interface;
for (const propName in Interface) {
// Modern event system doesn't use pooling.
if (!enableModernEventSystem) {
const Interface = this.constructor.Interface;
for (const propName in Interface) {
if (__DEV__) {
Object.defineProperty(
this,
propName,
getPooledWarningPropertyDefinition(propName, Interface[propName]),
);
} else {
this[propName] = null;
}
}
this.dispatchConfig = null;
this._targetInst = null;
this.nativeEvent = null;
this.isDefaultPrevented = functionThatReturnsFalse;
this.isPropagationStopped = functionThatReturnsFalse;
this._dispatchListeners = null;
this._dispatchInstances = null;
if (__DEV__) {
Object.defineProperty(
this,
propName,
getPooledWarningPropertyDefinition(propName, Interface[propName]),
'nativeEvent',
getPooledWarningPropertyDefinition('nativeEvent', null),
);
} else {
this[propName] = null;
}
}
this.dispatchConfig = null;
this._targetInst = null;
this.nativeEvent = null;
this.isDefaultPrevented = functionThatReturnsFalse;
this.isPropagationStopped = functionThatReturnsFalse;
if (!enableModernEventSystem) {
this._dispatchListeners = null;
this._dispatchInstances = null;
}
if (__DEV__) {
Object.defineProperty(
this,
'nativeEvent',
getPooledWarningPropertyDefinition('nativeEvent', null),
);
Object.defineProperty(
this,
'isDefaultPrevented',
getPooledWarningPropertyDefinition(
Object.defineProperty(
this,
'isDefaultPrevented',
functionThatReturnsFalse,
),
);
Object.defineProperty(
this,
'isPropagationStopped',
getPooledWarningPropertyDefinition(
getPooledWarningPropertyDefinition(
'isDefaultPrevented',
functionThatReturnsFalse,
),
);
Object.defineProperty(
this,
'isPropagationStopped',
functionThatReturnsFalse,
),
);
Object.defineProperty(
this,
'preventDefault',
getPooledWarningPropertyDefinition('preventDefault', () => {}),
);
Object.defineProperty(
this,
'stopPropagation',
getPooledWarningPropertyDefinition('stopPropagation', () => {}),
);
getPooledWarningPropertyDefinition(
'isPropagationStopped',
functionThatReturnsFalse,
),
);
Object.defineProperty(
this,
'preventDefault',
getPooledWarningPropertyDefinition('preventDefault', () => {}),
);
Object.defineProperty(
this,
'stopPropagation',
getPooledWarningPropertyDefinition('stopPropagation', () => {}),
);
}
}
},
});
Expand Down Expand Up @@ -303,18 +309,26 @@ function getPooledWarningPropertyDefinition(propName, getVal) {
}
}

function getPooledEvent(dispatchConfig, targetInst, nativeEvent, nativeInst) {
function createOrGetPooledEvent(
dispatchConfig,
targetInst,
nativeEvent,
nativeInst,
) {
const EventConstructor = this;
if (EventConstructor.eventPool.length) {
const instance = EventConstructor.eventPool.pop();
EventConstructor.call(
instance,
dispatchConfig,
targetInst,
nativeEvent,
nativeInst,
);
return instance;
// Modern event system doesn't use pooling.
if (!enableModernEventSystem) {
if (EventConstructor.eventPool.length) {
const instance = EventConstructor.eventPool.pop();
EventConstructor.call(
instance,
dispatchConfig,
targetInst,
nativeEvent,
nativeInst,
);
return instance;
}
}
return new EventConstructor(
dispatchConfig,
Expand All @@ -325,21 +339,28 @@ function getPooledEvent(dispatchConfig, targetInst, nativeEvent, nativeInst) {
}

function releasePooledEvent(event) {
const EventConstructor = this;
invariant(
event instanceof EventConstructor,
'Trying to release an event instance into a pool of a different type.',
);
event.destructor();
if (EventConstructor.eventPool.length < EVENT_POOL_SIZE) {
EventConstructor.eventPool.push(event);
// Modern event system doesn't use pooling.
if (!enableModernEventSystem) {
const EventConstructor = this;
invariant(
event instanceof EventConstructor,
'Trying to release an event instance into a pool of a different type.',
);
event.destructor();
if (EventConstructor.eventPool.length < EVENT_POOL_SIZE) {
EventConstructor.eventPool.push(event);
}
}
}

function addEventPoolingTo(EventConstructor) {
EventConstructor.eventPool = [];
EventConstructor.getPooled = getPooledEvent;
EventConstructor.release = releasePooledEvent;
EventConstructor.getPooled = createOrGetPooledEvent;

// Modern event system doesn't use pooling.
if (!enableModernEventSystem) {
EventConstructor.eventPool = [];
EventConstructor.release = releasePooledEvent;
}
}

export default SyntheticEvent;
5 changes: 1 addition & 4 deletions packages/react-dom/src/events/DOMModernPluginEventSystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,7 @@ export function dispatchEventsInBatch(dispatchQueue: DispatchQueue): void {
const dispatchQueueItem: DispatchQueueItem = dispatchQueue[i];
const {event, capture, bubble} = dispatchQueueItem;
executeDispatchesInOrder(event, capture, bubble);
// Release the event from the pool if needed
if (!event.isPersistent()) {
event.constructor.release(event);
}
// Modern event system doesn't use pooling.
}
// This would be a good time to rethrow if any of the event handlers threw.
rethrowCaughtError();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,27 @@ describe('DOMModernPluginEventSystem', () => {
endNativeEventListenerClearDown();
});

it('does not pool events', () => {
const buttonRef = React.createRef();
const log = [];
const onClick = jest.fn(e => log.push(e));

function Test() {
return <button ref={buttonRef} onClick={onClick} />;
}

ReactDOM.render(<Test />, container);

let buttonElement = buttonRef.current;
dispatchClickEvent(buttonElement);
expect(onClick).toHaveBeenCalledTimes(1);
dispatchClickEvent(buttonElement);
expect(onClick).toHaveBeenCalledTimes(2);
expect(log[0]).not.toBe(log[1]);
expect(log[0].type).toBe('click');
expect(log[1].type).toBe('click');
});

it('handle propagation of click events', () => {
const buttonRef = React.createRef();
const divRef = React.createRef();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

let React;
let ReactDOM;
const ReactFeatureFlags = require('shared/ReactFeatureFlags');

describe('SyntheticClipboardEvent', () => {
let container;
Expand Down Expand Up @@ -117,41 +118,43 @@ describe('SyntheticClipboardEvent', () => {
expect(expectedCount).toBe(3);
});

it('is able to `persist`', () => {
const persistentEvents = [];
const eventHandler = event => {
expect(event.isPersistent()).toBe(false);
event.persist();
expect(event.isPersistent()).toBe(true);
persistentEvents.push(event);
};

const div = ReactDOM.render(
<div
onCopy={eventHandler}
onCut={eventHandler}
onPaste={eventHandler}
/>,
container,
);

let event;
event = document.createEvent('Event');
event.initEvent('copy', true, true);
div.dispatchEvent(event);

event = document.createEvent('Event');
event.initEvent('cut', true, true);
div.dispatchEvent(event);

event = document.createEvent('Event');
event.initEvent('paste', true, true);
div.dispatchEvent(event);

expect(persistentEvents.length).toBe(3);
expect(persistentEvents[0].type).toBe('copy');
expect(persistentEvents[1].type).toBe('cut');
expect(persistentEvents[2].type).toBe('paste');
});
if (!ReactFeatureFlags.enableModernEventSystem) {
it('is able to `persist`', () => {
const persistentEvents = [];
const eventHandler = event => {
expect(event.isPersistent()).toBe(false);
event.persist();
expect(event.isPersistent()).toBe(true);
persistentEvents.push(event);
};

const div = ReactDOM.render(
<div
onCopy={eventHandler}
onCut={eventHandler}
onPaste={eventHandler}
/>,
container,
);

let event;
event = document.createEvent('Event');
event.initEvent('copy', true, true);
div.dispatchEvent(event);

event = document.createEvent('Event');
event.initEvent('cut', true, true);
div.dispatchEvent(event);

event = document.createEvent('Event');
event.initEvent('paste', true, true);
div.dispatchEvent(event);

expect(persistentEvents.length).toBe(3);
expect(persistentEvents[0].type).toBe('copy');
expect(persistentEvents[1].type).toBe('cut');
expect(persistentEvents[2].type).toBe('paste');
});
}
});
});
Loading