From 61e99eed8084ecaae1337ca7d6dfe9aa1b5c6f16 Mon Sep 17 00:00:00 2001 From: Ashoat Tevosyan Date: Tue, 26 Dec 2023 15:13:55 -0500 Subject: [PATCH] [lib] Introduce useDerivedObject for caching whole object when no values change Summary: We only need a subset of a `KeyserverInfoPartial` for constructing a keyserver call. Let's call that subset `KeyserverCallInfo`. We can introduce a selector that will cache a `KeyserverCallInfo` for a given `KeyserverInfoPartial`. But we still have a problem, because we still need a way to make sure the whole object of `KeyserverCallInfo`s doesn't change if a single `KeyserverCallInfo` changes. This `useDerivedObject` does that. However, we still have an issue owing to the fact that the `useDerivedObject` will cache on a per-hook-invocation level, whereas `bindCallKeyserverEndpointSelector` is caching globally. The following diff will resolve that. Depends on D10464 Test Plan: Before this stack, I was able to reproduce [ENG-3612](https://linear.app/comm/issue/ENG-3612/[native]-getting-huge-number-of-unhandled-promise-rejection-when) by going to the `ThreadSettings` screen in `native` while my local `keyserver` was down. After this stack, the issue no longer repros. I also compiled a release build of the iOS app to my phone to confirm that there were no regressions in TTI, or the time it takes to open a `MessageList` and go back to the `ChatThreadList`. Reviewers: inka, rohan Reviewed By: inka Subscribers: tomek Differential Revision: https://phab.comm.dev/D10465 --- lib/hooks/objects.js | 70 +++++++++++++++++++++++++++++++++ lib/utils/keyserver-call.js | 77 ++++++++++++++++++++++++++++++++----- 2 files changed, 137 insertions(+), 10 deletions(-) create mode 100644 lib/hooks/objects.js diff --git a/lib/hooks/objects.js b/lib/hooks/objects.js new file mode 100644 index 0000000000..26a2f01082 --- /dev/null +++ b/lib/hooks/objects.js @@ -0,0 +1,70 @@ +// @flow + +import * as React from 'react'; + +type CacheEntry = { + +inVal: I, + +outVal: O, + +derivationSelector: I => O, +}; + +function useDerivedObject( + object: { +[string]: I }, + createDerivationSelector: () => I => O, +): { +[string]: O } { + const cacheRef = React.useRef>>(new Map()); + const prevCreateDerivationSelector = React.useRef<() => I => O>( + createDerivationSelector, + ); + const prevResultRef = React.useRef(); + + return React.useMemo(() => { + if (prevCreateDerivationSelector.current !== createDerivationSelector) { + cacheRef.current = new Map(); + prevCreateDerivationSelector.current = createDerivationSelector; + } + + const cache = cacheRef.current; + + const newCache = new Map>(); + let changeOccurred = Object.keys(object).length !== cache.size; + + const result: { [string]: O } = {}; + for (const key in object) { + const inVal = object[key]; + + const cacheEntry = cache.get(key); + if (!cacheEntry) { + changeOccurred = true; + const derivationSelector = createDerivationSelector(); + const outVal = derivationSelector(inVal); + newCache.set(key, { inVal, outVal, derivationSelector }); + result[key] = outVal; + continue; + } + + if (inVal === cacheEntry.inVal) { + newCache.set(key, cacheEntry); + result[key] = cacheEntry.outVal; + continue; + } + + const { derivationSelector } = cacheEntry; + const outVal = derivationSelector(inVal); + if (outVal !== cacheEntry.outVal) { + changeOccurred = true; + } + newCache.set(key, { inVal, outVal, derivationSelector }); + result[key] = outVal; + } + cacheRef.current = newCache; + + if (!changeOccurred && prevResultRef.current) { + return prevResultRef.current; + } + prevResultRef.current = result; + return result; + }, [object, createDerivationSelector]); +} + +export { useDerivedObject }; diff --git a/lib/utils/keyserver-call.js b/lib/utils/keyserver-call.js index 8b3870a9f4..7ab3788773 100644 --- a/lib/utils/keyserver-call.js +++ b/lib/utils/keyserver-call.js @@ -12,6 +12,7 @@ import type { } from './call-server-endpoint.js'; import { promiseAll } from './promises.js'; import { useSelector, useDispatch } from './redux-utils.js'; +import { useDerivedObject } from '../hooks/objects.js'; import type { PlatformDetails } from '../types/device-types.js'; import type { Endpoint } from '../types/endpoints.js'; import type { KeyserverInfo } from '../types/keyserver-types.js'; @@ -78,10 +79,52 @@ type KeyserverInfoPartial = $ReadOnly<{ +urlPrefix: $PropertyType, }>; +type KeyserverCallInfo = { + +cookie: ?string, + +urlPrefix: string, + +sessionID: ?string, + +isSocketConnected: boolean, + +lastCommunicatedPlatformDetails: ?PlatformDetails, +}; + +const createKeyserverCallSelector: () => KeyserverInfoPartial => KeyserverCallInfo = + () => + createSelector( + (keyserverInfo: KeyserverInfoPartial) => keyserverInfo.cookie, + (keyserverInfo: KeyserverInfoPartial) => keyserverInfo.urlPrefix, + (keyserverInfo: KeyserverInfoPartial) => keyserverInfo.sessionID, + (keyserverInfo: KeyserverInfoPartial) => + keyserverInfo.connection?.status === 'connected', + (keyserverInfo: KeyserverInfoPartial) => + keyserverInfo.lastCommunicatedPlatformDetails, + ( + cookie: ?string, + urlPrefix: string, + sessionID: ?string, + isSocketConnected: boolean, + lastCommunicatedPlatformDetails: ?PlatformDetails, + ) => ({ + cookie, + urlPrefix, + sessionID, + isSocketConnected, + lastCommunicatedPlatformDetails, + }), + ); + +function useKeyserverCallInfos(keyserverInfos: { + +[keyserverID: string]: KeyserverInfoPartial, +}): { +[keyserverID: string]: KeyserverCallInfo } { + return useDerivedObject( + keyserverInfos, + createKeyserverCallSelector, + ); +} + type BindKeyserverCallParams = { +dispatch: Dispatch, +currentUserInfo: ?CurrentUserInfo, - +keyserverInfos: { +[keyserverID: string]: KeyserverInfoPartial }, + +keyserverCallInfos: { +[keyserverID: string]: KeyserverCallInfo }, }; const bindCallKeyserverEndpointSelector: BindKeyserverCallParams => < @@ -92,11 +135,11 @@ const bindCallKeyserverEndpointSelector: BindKeyserverCallParams => < ) => Args => Promise = createSelector( (state: BindKeyserverCallParams) => state.dispatch, (state: BindKeyserverCallParams) => state.currentUserInfo, - (state: BindKeyserverCallParams) => state.keyserverInfos, + (state: BindKeyserverCallParams) => state.keyserverCallInfos, ( dispatch: Dispatch, currentUserInfo: ?CurrentUserInfo, - keyserverInfos: { +[keyserverID: string]: KeyserverInfoPartial }, + keyserverCallInfos: { +[keyserverID: string]: KeyserverCallInfo }, ) => { return _memoize( ( @@ -112,9 +155,9 @@ const bindCallKeyserverEndpointSelector: BindKeyserverCallParams => < cookie, urlPrefix, sessionID, - connection, + isSocketConnected, lastCommunicatedPlatformDetails, - } = keyserverInfos[keyserverID]; + } = keyserverCallInfos[keyserverID]; const boundCallServerEndpoint = createBoundServerCallsSelector( keyserverID, @@ -124,7 +167,7 @@ const bindCallKeyserverEndpointSelector: BindKeyserverCallParams => < cookie, urlPrefix, sessionID, - isSocketConnected: connection?.status === 'connected', + isSocketConnected, lastCommunicatedPlatformDetails, keyserverID, }); @@ -142,29 +185,43 @@ const bindCallKeyserverEndpointSelector: BindKeyserverCallParams => < } return promiseAll(promises); }; - const keyserverIDs = Object.keys(keyserverInfos); + const keyserverIDs = Object.keys(keyserverCallInfos); return keyserverCall(callKeyserverEndpoint, keyserverIDs); }, ); }, ); -export type KeyserverCallParamOverride = Partial; +export type KeyserverCallParamOverride = Partial<{ + +dispatch: Dispatch, + +currentUserInfo: ?CurrentUserInfo, + +keyserverInfos: { +[keyserverID: string]: KeyserverInfoPartial }, +}>; function useKeyserverCall( keyserverCall: ActionFunc, paramOverride?: ?KeyserverCallParamOverride, ): Args => Promise { const dispatch = useDispatch(); + const currentUserInfo = useSelector(state => state.currentUserInfo); + const keyserverInfos = useSelector( state => state.keyserverStore.keyserverInfos, ); - const currentUserInfo = useSelector(state => state.currentUserInfo); - const bindCallKeyserverEndpointToAction = bindCallKeyserverEndpointSelector({ + const baseCombinedInfo = { dispatch, keyserverInfos, currentUserInfo, ...paramOverride, + }; + + const { keyserverInfos: keyserverInfoPartials, ...restCombinedInfo } = + baseCombinedInfo; + const keyserverCallInfos = useKeyserverCallInfos(keyserverInfoPartials); + + const bindCallKeyserverEndpointToAction = bindCallKeyserverEndpointSelector({ + ...restCombinedInfo, + keyserverCallInfos, }); return React.useMemo( () => bindCallKeyserverEndpointToAction(keyserverCall),