From f4866537caff3e47f6fed5ae16b7cdd2f4957a98 Mon Sep 17 00:00:00 2001 From: Jan Kassens Date: Tue, 6 Apr 2021 07:24:00 -0700 Subject: [PATCH] remove RelayProfiler from RelayModernStore.prototype.lookup and RelayProfiler.instrumentMethods Reviewed By: josephsavona Differential Revision: D27566450 fbshipit-source-id: 732058094b5924f4be2c0020288894b034112b6e --- .../relay-runtime/store/RelayModernStore.js | 5 - packages/relay-runtime/util/RelayProfiler.js | 28 - .../util/__mocks__/RelayProfiler.js | 5 - .../util/__tests__/RelayProfiler-test.js | 496 +++++++++--------- 4 files changed, 238 insertions(+), 296 deletions(-) diff --git a/packages/relay-runtime/store/RelayModernStore.js b/packages/relay-runtime/store/RelayModernStore.js index f2377897685f1..dd91dfb9417ee 100644 --- a/packages/relay-runtime/store/RelayModernStore.js +++ b/packages/relay-runtime/store/RelayModernStore.js @@ -16,7 +16,6 @@ const DataChecker = require('./DataChecker'); const RelayFeatureFlags = require('../util/RelayFeatureFlags'); const RelayModernRecord = require('./RelayModernRecord'); const RelayOptimisticRecordSource = require('./RelayOptimisticRecordSource'); -const RelayProfiler = require('../util/RelayProfiler'); const RelayReader = require('./RelayReader'); const RelayReferenceMarker = require('./RelayReferenceMarker'); const RelayStoreReactFlightUtils = require('./RelayStoreReactFlightUtils'); @@ -770,8 +769,4 @@ function getAvailabilityStatus( return {status: 'available', fetchTime: operationFetchTime ?? null}; } -RelayProfiler.instrumentMethods(RelayModernStore.prototype, { - lookup: 'RelayModernStore.prototype.lookup', -}); - module.exports = RelayModernStore; diff --git a/packages/relay-runtime/util/RelayProfiler.js b/packages/relay-runtime/util/RelayProfiler.js index 38171892caf9d..3c2326989d8c3 100644 --- a/packages/relay-runtime/util/RelayProfiler.js +++ b/packages/relay-runtime/util/RelayProfiler.js @@ -66,34 +66,6 @@ const shouldInstrument = name => { * if `__DEV__` is true. This should be used for very hot functions. */ const RelayProfiler = { - /** - * Instruments methods on a class or object. This re-assigns the method in - * order to preserve function names in stack traces (which are detected by - * modern debuggers via heuristics). Example usage: - * - * const RelayStore = { primeCache: function() {...} }; - * RelayProfiler.instrumentMethods(RelayStore, { - * primeCache: 'RelayStore.primeCache' - * }); - * - * RelayStore.primeCache.attachHandler(...); - * - * As a result, the methods will be replaced by wrappers that provide the - * `attachHandler` and `detachHandler` methods. - */ - instrumentMethods( - object: Function | Object, - names: {[key: string]: string, ...}, - ): void { - for (const key in names) { - if (names.hasOwnProperty(key)) { - if (typeof object[key] === 'function') { - object[key] = RelayProfiler.instrument(names[key], object[key]); - } - } - } - }, - /** * Wraps the supplied function with one that provides the `attachHandler` and * `detachHandler` methods. Example usage: diff --git a/packages/relay-runtime/util/__mocks__/RelayProfiler.js b/packages/relay-runtime/util/__mocks__/RelayProfiler.js index e65eac9f8a960..c1a0464f4187e 100644 --- a/packages/relay-runtime/util/__mocks__/RelayProfiler.js +++ b/packages/relay-runtime/util/__mocks__/RelayProfiler.js @@ -10,11 +10,6 @@ 'use strict'; const RelayProfiler = { - instrumentMethods: jest.fn((object, names) => { - for (const [key, name] of Object.entries(names)) { - object[key] = RelayProfiler.instrument(name, object[key]); - } - }), instrument: jest.fn((name, handler) => { handler.attachHandler = () => {}; handler.detachHandler = () => {}; diff --git a/packages/relay-runtime/util/__tests__/RelayProfiler-test.js b/packages/relay-runtime/util/__tests__/RelayProfiler-test.js index 19a31064ab352..a79035c13ab41 100644 --- a/packages/relay-runtime/util/__tests__/RelayProfiler-test.js +++ b/packages/relay-runtime/util/__tests__/RelayProfiler-test.js @@ -12,322 +12,302 @@ const RelayProfiler = require('../RelayProfiler'); -describe('RelayProfiler', function() { - const DEV = __DEV__; - - let mockMethod; - let mockObject; - const mockDisableDEV = () => { - global.__DEV__ = 0; +const DEV = __DEV__; + +let mockMethod; +let mockObject; +const mockDisableDEV = () => { + global.__DEV__ = 0; +}; + +beforeEach(() => { + jest.resetModules(); + + mockMethod = jest.fn(); + const mockMethod2 = jest.fn(); + mockObject = { + mockMethod: RelayProfiler.instrument('mock', mockMethod), + mockMethod2: RelayProfiler.instrument('mock2', mockMethod2), }; +}); - beforeEach(() => { - jest.resetModules(); +afterEach(() => { + global.__DEV__ = DEV; +}); - mockMethod = jest.fn(); - const mockMethod2 = jest.fn(); - mockObject = { - mockMethod: RelayProfiler.instrument('mock', mockMethod), - mockMethod2: RelayProfiler.instrument('mock2', mockMethod2), - }; - }); +describe('instance', () => { + it('preserves context, arguments, and return value', () => { + const expectedArgument = {}; + const expectedContext = mockObject; + const expectedReturnValue = {}; - afterEach(() => { - global.__DEV__ = DEV; - }); + mockMethod.mockImplementation(function(actualArgument) { + expect(actualArgument).toBe(expectedArgument); + expect(this).toBe(expectedContext); + return expectedReturnValue; + }); - describe('instance', () => { - it('preserves context, arguments, and return value', () => { - const expectedArgument = {}; - const expectedContext = mockObject; - const expectedReturnValue = {}; + const actualReturnValue = mockObject.mockMethod(expectedArgument); - mockMethod.mockImplementation(function(actualArgument) { - expect(actualArgument).toBe(expectedArgument); - expect(this).toBe(expectedContext); - return expectedReturnValue; - }); + expect(actualReturnValue).toBe(expectedReturnValue); + }); - const actualReturnValue = mockObject.mockMethod(expectedArgument); + it('invokes attached handlers', () => { + const actualOrdering = []; - expect(actualReturnValue).toBe(expectedReturnValue); + mockMethod.mockImplementation(() => { + actualOrdering.push('mockMethod'); }); - it('does not create wrapper methods unless they exist on the instance', () => { - class Container { - methodThatExists() {} - } - RelayProfiler.instrumentMethods(Container.prototype, { - methodThatExists: 'Container.prototype.methodThatExists', - methodThatDoesNotExist: 'Container.prototype.methodThatDoesNotExist', - }); - const container = new Container(); - expect(typeof container.methodThatExists).toBe('function'); - expect(typeof container.methodThatDoesNotExist).toBe('undefined'); + mockObject.mockMethod.attachHandler((name, callback) => { + expect(name).toBe('mock'); + actualOrdering.push('beforeCallback'); + callback(); + actualOrdering.push('afterCallback'); }); - it('invokes attached handlers', () => { - const actualOrdering = []; + mockObject.mockMethod(); - mockMethod.mockImplementation(() => { - actualOrdering.push('mockMethod'); - }); + expect(actualOrdering).toEqual([ + 'beforeCallback', + 'mockMethod', + 'afterCallback', + ]); + }); - mockObject.mockMethod.attachHandler((name, callback) => { - expect(name).toBe('mock'); - actualOrdering.push('beforeCallback'); - callback(); - actualOrdering.push('afterCallback'); - }); + it('invokes nested attached handlers', () => { + const actualOrdering = []; - mockObject.mockMethod(); + mockMethod.mockImplementation(() => { + actualOrdering.push('0: mockMethod'); + }); - expect(actualOrdering).toEqual([ - 'beforeCallback', - 'mockMethod', - 'afterCallback', - ]); + mockObject.mockMethod.attachHandler((name, callback) => { + expect(name).toBe('mock'); + actualOrdering.push('1: beforeCallback'); + callback(); + actualOrdering.push('1: afterCallback'); }); - it('invokes nested attached handlers', () => { - const actualOrdering = []; + mockObject.mockMethod.attachHandler((name, callback) => { + expect(name).toBe('mock'); + actualOrdering.push('2: beforeCallback'); + callback(); + actualOrdering.push('2: afterCallback'); + }); - mockMethod.mockImplementation(() => { - actualOrdering.push('0: mockMethod'); - }); + mockObject.mockMethod(); - mockObject.mockMethod.attachHandler((name, callback) => { - expect(name).toBe('mock'); - actualOrdering.push('1: beforeCallback'); - callback(); - actualOrdering.push('1: afterCallback'); - }); + expect(actualOrdering).toEqual([ + '2: beforeCallback', + '1: beforeCallback', + '0: mockMethod', + '1: afterCallback', + '2: afterCallback', + ]); + }); - mockObject.mockMethod.attachHandler((name, callback) => { - expect(name).toBe('mock'); - actualOrdering.push('2: beforeCallback'); - callback(); - actualOrdering.push('2: afterCallback'); - }); + it('does not invoke detached handlers', () => { + const mockHandler = jest.fn().mockImplementation((name, callback) => { + callback(); + }); - mockObject.mockMethod(); + mockObject.mockMethod.attachHandler(mockHandler); + mockObject.mockMethod.detachHandler(mockHandler); + mockObject.mockMethod(); - expect(actualOrdering).toEqual([ - '2: beforeCallback', - '1: beforeCallback', - '0: mockMethod', - '1: afterCallback', - '2: afterCallback', - ]); - }); + expect(mockHandler).not.toBeCalled(); + }); - it('does not invoke detached handlers', () => { - const mockHandler = jest.fn().mockImplementation((name, callback) => { - callback(); - }); + it('throws if callback is not invoked by handler', () => { + mockObject.mockMethod.attachHandler(jest.fn()); - mockObject.mockMethod.attachHandler(mockHandler); - mockObject.mockMethod.detachHandler(mockHandler); + expect(() => { mockObject.mockMethod(); + }).toThrowError('RelayProfiler: Handler did not invoke original function.'); + }); - expect(mockHandler).not.toBeCalled(); - }); + it('ignores names starting with "@" unless __DEV__', () => { + mockDisableDEV(); - it('throws if callback is not invoked by handler', () => { - mockObject.mockMethod.attachHandler(jest.fn()); + mockMethod = jest.fn(); + mockObject = {mockMethod: RelayProfiler.instrument('@mock', mockMethod)}; - expect(() => { - mockObject.mockMethod(); - }).toThrowError( - 'RelayProfiler: Handler did not invoke original function.', - ); - }); + expect(mockObject.mockMethod).toBe(mockMethod); + expect(() => { + mockObject.mockMethod.attachHandler(); + mockObject.mockMethod.detachHandler(); + }).not.toThrow(); + }); - it('ignores names starting with "@" unless __DEV__', () => { - mockDisableDEV(); + it('instruments names without "@" when not in __DEV__', () => { + mockDisableDEV(); - mockMethod = jest.fn(); - mockObject = {mockMethod: RelayProfiler.instrument('@mock', mockMethod)}; + mockMethod = jest.fn(); + mockObject = {mockMethod: RelayProfiler.instrument('mock', mockMethod)}; - expect(mockObject.mockMethod).toBe(mockMethod); - expect(() => { - mockObject.mockMethod.attachHandler(); - mockObject.mockMethod.detachHandler(); - }).not.toThrow(); - }); + expect(mockObject.mockMethod).not.toBe(mockMethod); + }); +}); - it('instruments names without "@" when not in __DEV__', () => { - mockDisableDEV(); +describe('aggregate', () => { + it('invokes aggregate handlers first', () => { + const actualOrdering = []; - mockMethod = jest.fn(); - mockObject = {mockMethod: RelayProfiler.instrument('mock', mockMethod)}; + mockMethod.mockImplementation(() => { + actualOrdering.push('0: mockMethod'); + }); - expect(mockObject.mockMethod).not.toBe(mockMethod); + mockObject.mockMethod.attachHandler((name, callback) => { + actualOrdering.push('1: beforeCallback'); + callback(); + actualOrdering.push('1: afterCallback'); }); - }); - describe('aggregate', () => { - it('invokes aggregate handlers first', () => { - const actualOrdering = []; - - mockMethod.mockImplementation(() => { - actualOrdering.push('0: mockMethod'); - }); - - mockObject.mockMethod.attachHandler((name, callback) => { - actualOrdering.push('1: beforeCallback'); - callback(); - actualOrdering.push('1: afterCallback'); - }); - - RelayProfiler.attachAggregateHandler('mock', (name, callback) => { - expect(name).toBe('mock'); - actualOrdering.push('3: beforeCallback (aggregate)'); - callback(); - actualOrdering.push('3: afterCallback (aggregate)'); - }); - - RelayProfiler.attachAggregateHandler('*', (name, callback) => { - actualOrdering.push('5: beforeCallback (aggregate *): ' + name); - callback(); - actualOrdering.push('5: afterCallback (aggregate *): ' + name); - }); - - RelayProfiler.attachAggregateHandler('mock', (name, callback) => { - expect(name).toBe('mock'); - actualOrdering.push('4: beforeCallback (aggregate)'); - callback(); - actualOrdering.push('4: afterCallback (aggregate)'); - }); - - mockObject.mockMethod.attachHandler((name, callback) => { - actualOrdering.push('2: beforeCallback'); - callback(); - actualOrdering.push('2: afterCallback'); - }); + RelayProfiler.attachAggregateHandler('mock', (name, callback) => { + expect(name).toBe('mock'); + actualOrdering.push('3: beforeCallback (aggregate)'); + callback(); + actualOrdering.push('3: afterCallback (aggregate)'); + }); - mockObject.mockMethod(); - mockObject.mockMethod2(); - - expect(actualOrdering).toEqual([ - '5: beforeCallback (aggregate *): mock', - '4: beforeCallback (aggregate)', - '3: beforeCallback (aggregate)', - '2: beforeCallback', - '1: beforeCallback', - '0: mockMethod', - '1: afterCallback', - '2: afterCallback', - '3: afterCallback (aggregate)', - '4: afterCallback (aggregate)', - '5: afterCallback (aggregate *): mock', - '5: beforeCallback (aggregate *): mock2', - '5: afterCallback (aggregate *): mock2', - ]); + RelayProfiler.attachAggregateHandler('*', (name, callback) => { + actualOrdering.push('5: beforeCallback (aggregate *): ' + name); + callback(); + actualOrdering.push('5: afterCallback (aggregate *): ' + name); }); - it('aggregates methods instrumented after being attached', () => { - const mockHandler = jest.fn().mockImplementation((name, callback) => { - callback(); - }); - RelayProfiler.attachAggregateHandler('mockFuture', mockHandler); + RelayProfiler.attachAggregateHandler('mock', (name, callback) => { + expect(name).toBe('mock'); + actualOrdering.push('4: beforeCallback (aggregate)'); + callback(); + actualOrdering.push('4: afterCallback (aggregate)'); + }); - const mockFutureMethod = RelayProfiler.instrument( - 'mockFuture', - mockMethod, - ); + mockObject.mockMethod.attachHandler((name, callback) => { + actualOrdering.push('2: beforeCallback'); + callback(); + actualOrdering.push('2: afterCallback'); + }); - expect(mockHandler).not.toBeCalled(); - mockFutureMethod(); - expect(mockHandler).toBeCalled(); + mockObject.mockMethod(); + mockObject.mockMethod2(); + + expect(actualOrdering).toEqual([ + '5: beforeCallback (aggregate *): mock', + '4: beforeCallback (aggregate)', + '3: beforeCallback (aggregate)', + '2: beforeCallback', + '1: beforeCallback', + '0: mockMethod', + '1: afterCallback', + '2: afterCallback', + '3: afterCallback (aggregate)', + '4: afterCallback (aggregate)', + '5: afterCallback (aggregate *): mock', + '5: beforeCallback (aggregate *): mock2', + '5: afterCallback (aggregate *): mock2', + ]); + }); + + it('aggregates methods instrumented after being attached', () => { + const mockHandler = jest.fn().mockImplementation((name, callback) => { + callback(); }); + RelayProfiler.attachAggregateHandler('mockFuture', mockHandler); - it('detaches aggregate handlers', () => { - const mockHandler = jest.fn().mockImplementation((name, callback) => { - callback(); - }); + const mockFutureMethod = RelayProfiler.instrument('mockFuture', mockMethod); - RelayProfiler.attachAggregateHandler('mock', mockHandler); - RelayProfiler.detachAggregateHandler('mock', mockHandler); - mockObject.mockMethod(); + expect(mockHandler).not.toBeCalled(); + mockFutureMethod(); + expect(mockHandler).toBeCalled(); + }); - expect(mockHandler).not.toBeCalled(); + it('detaches aggregate handlers', () => { + const mockHandler = jest.fn().mockImplementation((name, callback) => { + callback(); }); + + RelayProfiler.attachAggregateHandler('mock', mockHandler); + RelayProfiler.detachAggregateHandler('mock', mockHandler); + mockObject.mockMethod(); + + expect(mockHandler).not.toBeCalled(); }); +}); - describe('profile', () => { - it('invokes attached profile handlers', () => { - const actualOrdering = []; - - RelayProfiler.attachProfileHandler('mockBehavior', name => { - expect(name).toBe('mockBehavior'); - actualOrdering.push('1: beforeEnd'); - return () => { - actualOrdering.push('1: afterEnd'); - }; - }); - - RelayProfiler.attachProfileHandler('mockBehavior', name => { - expect(name).toBe('mockBehavior'); - actualOrdering.push('2: beforeEnd'); - return () => { - actualOrdering.push('2: afterEnd'); - }; - }); - - const profiler = RelayProfiler.profile('mockBehavior'); - - expect(actualOrdering).toEqual(['2: beforeEnd', '1: beforeEnd']); - - profiler.stop(); - - expect(actualOrdering).toEqual([ - '2: beforeEnd', - '1: beforeEnd', - '1: afterEnd', - '2: afterEnd', - ]); +describe('profile', () => { + it('invokes attached profile handlers', () => { + const actualOrdering = []; + + RelayProfiler.attachProfileHandler('mockBehavior', name => { + expect(name).toBe('mockBehavior'); + actualOrdering.push('1: beforeEnd'); + return () => { + actualOrdering.push('1: afterEnd'); + }; }); - it('does not invoke detached profile handlers', () => { - const mockStop = jest.fn(); - const mockStart = jest.fn(() => mockStop); + RelayProfiler.attachProfileHandler('mockBehavior', name => { + expect(name).toBe('mockBehavior'); + actualOrdering.push('2: beforeEnd'); + return () => { + actualOrdering.push('2: afterEnd'); + }; + }); - RelayProfiler.attachProfileHandler('mockBehavior', mockStart); - RelayProfiler.detachProfileHandler('mockBehavior', mockStart); - RelayProfiler.profile('mockBehavior'); + const profiler = RelayProfiler.profile('mockBehavior'); - expect(mockStop).not.toBeCalled(); - expect(mockStart).not.toBeCalled(); - }); + expect(actualOrdering).toEqual(['2: beforeEnd', '1: beforeEnd']); - it('passes state to each profile handler', () => { - const mockStop = jest.fn(); - const mockStart = jest.fn(() => mockStop); - const state = {}; + profiler.stop(); - RelayProfiler.attachProfileHandler('mockBehavior', mockStart); - const profiler = RelayProfiler.profile('mockBehavior', state); - profiler.stop(); + expect(actualOrdering).toEqual([ + '2: beforeEnd', + '1: beforeEnd', + '1: afterEnd', + '2: afterEnd', + ]); + }); - expect(mockStart).toBeCalledWith('mockBehavior', state); - expect(mockStop).toBeCalled(); - expect(mockStop.mock.calls[0].length).toBe(1); - expect(mockStop.mock.calls[0][0]).toEqual(undefined); - }); + it('does not invoke detached profile handlers', () => { + const mockStop = jest.fn(); + const mockStart = jest.fn(() => mockStop); - it('passes error to each stop handler', () => { - const mockStop = jest.fn(); - const mockStart = jest.fn(() => mockStop); - const state = {}; + RelayProfiler.attachProfileHandler('mockBehavior', mockStart); + RelayProfiler.detachProfileHandler('mockBehavior', mockStart); + RelayProfiler.profile('mockBehavior'); - RelayProfiler.attachProfileHandler('mockBehavior', mockStart); - const profiler = RelayProfiler.profile('mockBehavior', state); - const error = new Error(); - profiler.stop(error); + expect(mockStop).not.toBeCalled(); + expect(mockStart).not.toBeCalled(); + }); - expect(mockStart).toBeCalledWith('mockBehavior', state); - expect(mockStop).toBeCalledWith(error); - }); + it('passes state to each profile handler', () => { + const mockStop = jest.fn(); + const mockStart = jest.fn(() => mockStop); + const state = {}; + + RelayProfiler.attachProfileHandler('mockBehavior', mockStart); + const profiler = RelayProfiler.profile('mockBehavior', state); + profiler.stop(); + + expect(mockStart).toBeCalledWith('mockBehavior', state); + expect(mockStop).toBeCalled(); + expect(mockStop.mock.calls[0].length).toBe(1); + expect(mockStop.mock.calls[0][0]).toEqual(undefined); + }); + + it('passes error to each stop handler', () => { + const mockStop = jest.fn(); + const mockStart = jest.fn(() => mockStop); + const state = {}; + + RelayProfiler.attachProfileHandler('mockBehavior', mockStart); + const profiler = RelayProfiler.profile('mockBehavior', state); + const error = new Error(); + profiler.stop(error); + + expect(mockStart).toBeCalledWith('mockBehavior', state); + expect(mockStop).toBeCalledWith(error); }); });