Page MenuHomePhabricator
Differential D8514

[keyserver] Filter fetchThreadInfos queries to only look at viewer's threads
ClosedPublic
Actions

Authored by ashoat on Jul 16 2023, 4:30 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 20, 4:14 PM
Unknown Object (File)
Sun, Apr 20, 2:15 PM
Unknown Object (File)
Sat, Apr 19, 10:03 PM
Unknown Object (File)
Fri, Apr 18, 11:01 PM
Unknown Object (File)
Mar 18 2025, 6:51 PM
Unknown Object (File)
Mar 16 2025, 11:36 PM
Unknown Object (File)
Mar 12 2025, 8:36 AM
Unknown Object (File)
Mar 11 2025, 10:32 PM
View All Files
Subscribers
None

Details

Reviewers
tomek
atul
Commits
rCOMMc92d32ab3b1b: [keyserver] Filter fetchThreadInfos queries to only look at viewer's threads
Summary

This attempts strategy 3 outlined in ENG-3877.

Depends on D8513

Test Plan

I started this task by testing various queries in the production environment to check performance.

After implementing the change, I used this patch to check performance on the JS side in my dev environment, and to output raw queries so I could test their performance in production: https://gist.github.com/Ashoat/220bb35da34ac7820867ac874d87beab

I found that for all users other than me, this change significantly sped up fetchThreadInfos.

I then deployed this to production to test performance more reliably. Hopefully so far no issues!

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Revision Contents
Changeset List

PathSize
keyserver/
src/
fetchers/
68 lines

Diff 28749

View Options

keyserver/src/fetchers/thread-fetchers.js

// @flow // @flow
import invariant from 'invariant'; import invariant from 'invariant';
import * as React from 'react'; import * as React from 'react';
import { View } from 'react-native'; import { View } from 'react-native';
import { ScrollView } from 'react-native-gesture-handler'; import { ScrollView } from 'react-native-gesture-handler';
import { fetchPinnedMessages } from 'lib/actions/message-actions.js'; import { fetchPinnedMessages } from 'lib/actions/message-actions.js';
import { messageListData } from 'lib/selectors/chat-selectors.js'; import { messageListData } from 'lib/selectors/chat-selectors.js';
import { createMessageInfo } from 'lib/shared/message-utils.js'; import { createMessageInfo } from 'lib/shared/message-utils.js';
import { isComposableMessageType } from 'lib/types/message-types.js';
import type { ThreadInfo } from 'lib/types/thread-types.js'; import type { ThreadInfo } from 'lib/types/thread-types.js';
import { useServerCall } from 'lib/utils/action-utils.js'; import { useServerCall } from 'lib/utils/action-utils.js';
import { useHeightMeasurer } from './chat-context.js'; import { useHeightMeasurer } from './chat-context.js';
import type { ChatNavigationProp } from './chat.react'; import type { ChatNavigationProp } from './chat.react';
import MessageResult from './message-result.react.js'; import MessageResult from './message-result.react.js';
import type { NavigationRoute } from '../navigation/route-names'; import type { NavigationRoute } from '../navigation/route-names';
import { useSelector } from '../redux/redux-utils.js'; import { useSelector } from '../redux/redux-utils.js';
▲ Show 20 Lines • Show All 46 Lines • ▼ Show 20 Lines const chatMessageInfos = useSelector(
messageListData(threadInfo.id, translatedMessageResults), messageListData(threadInfo.id, translatedMessageResults),
); );
const sortedUniqueChatMessageInfoItems = React.useMemo(() => { const sortedUniqueChatMessageInfoItems = React.useMemo(() => {
if (!chatMessageInfos) { if (!chatMessageInfos) {
return []; return [];
} }
const pinnedMessageIDs = new Set(); const pinnedMessageIDs = new Set();
translatedMessageResults.forEach(item => pinnedMessageIDs.add(item.id)); translatedMessageResults.forEach(item => pinnedMessageIDs.add(item.id));
inkaUnsubmitted
Not Done
Inline Actions

We could probably go back to using isPinned property here. The thing we undid in D8097. This would make the code more readable, but it's up to you

inka: We could probably go back to using `isPinned` property here. The thing we undid in D8097. This…
const chatMessageInfoItems = chatMessageInfos.filter( const chatMessageInfoItems = chatMessageInfos.filter(
item => item =>
item.itemType === 'message' && item.itemType === 'message' &&
pinnedMessageIDs.has(item.messageInfo.id), pinnedMessageIDs.has(item.messageInfo.id) &&
isComposableMessageType(item.messageInfo.type),
rohanAuthorUnsubmitted
Done
Inline Actions

As mentioned in the description, this is because toggle_pin messages are visible robotext, so if we don't filter those out then they will become visible in the message results modal. This is done here, after the ChatMessageInfos have been created, so we can accurately use the updated fetched related messages to determine the isPinned flag. If we filtered this earlier, then we'd be in the same situation as before

This also is fine because we only allow the user to pin composable message types so we won't exclude anything from the view that we need to show.

rohan: As mentioned in the description, this is because toggle_pin messages are visible robotext, so…
); );
// By the nature of using messageListData and passing in // By the nature of using messageListData and passing in
// the desired translatedMessageResults as additional // the desired translatedMessageResults as additional
// messages, we will have duplicate ChatMessageInfoItems. // messages, we will have duplicate ChatMessageInfoItems.
const uniqueChatMessageInfoItemsMap = new Map(); const uniqueChatMessageInfoItemsMap = new Map();
chatMessageInfoItems.forEach( chatMessageInfoItems.forEach(
item => item =>
▲ Show 20 Lines • Show All 93 Lines • Show Last 20 Lines