melvin-bot[bot]
commented
9 months ago Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
Closed hayata-suenaga closed 5 months ago
melvin-bot[bot]
commented
9 months ago Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
lschurr
commented
9 months ago Hey @hayata-suenaga - should this be External for contributors to work on or are we keeping this Internal?
melvin-bot[bot]
commented
9 months ago Job added to Upwork: https://www.upwork.com/jobs/~01e122c7e0ae0d76b5
melvin-bot[bot]
commented
9 months ago Triggered auto assignment to Contributor-plus team member for initial proposal review - @cubuspl42 (External)
hayata-suenaga
commented
9 months ago Hey @hayata-suenaga - should this be External for contributors to work on or are we keeping this Internal?
I'll keep this external for now. Thank you for reminding me of it 😄
hurali97
commented
9 months ago @mountiny can you assign this to me ? 👋
melvin-bot[bot]
commented
9 months ago 📣 @cubuspl42 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!
hayata-suenaga
commented
9 months ago @hurali97 I changed the issue description of the original post of this issue to include another related issue reported about the App becoming unresponsive. Please check the issue description again and let me know if you can handle the added issue. 🙇
hurali97
commented
9 months ago Hey @hayata-suenaga !
I took a look at the issue description and it's clear 👍 I tried to reproduce and locate the root cause but this one's tricky to reproduce. However, I found that switching between chat list and workspace is sometimes really slow. After locating the stack traces for these I found that most of the points are re-usable from the App Startup Audit on-going here. After adding the optimization code from these points, the switching between chat list and workspace is better than before by margins.
However, with the fixes I left the Expensify tab open for a few minutes switched to other tab and when I came back to Expensify and reloaded the page then switched to Workspace list, I got the Unresponsiveness Alert, even with the fixes applied from the above step. It's clear that the fixes doesn't solve the root cause which is apparently related to memory consumption.
I observed the memory usage whilst switching between chat list and workspaces a bunch of time the memory consumption gets to more than 2000 MB❗Upon analysing the memory allocation trace most of the allocation is consumed by getOrderedReportIDs.
I will analyse this further tomorrow and keep y'all posted 👍
hayata-suenaga
commented
9 months ago Upon analysing the memory allocation trace most of the allocation is consumed by getOrderedReportIDs.
I don't fully remember what the getOrderedReportIDs function does. But I remember that the app identifies which workspace items should include RBR/GBR when the Workspace Switcher displays the list of workspaces (screenshot below). It does so by reviewing all reports for each workspace to determine if any reports or chats require the user's attention, signified by RBR/GBR. This process might consume a significant amount of memory.
hurali97
commented
9 months ago @hayata-suenaga The getOrderedReportIDs is the one which prepares the data for LHN. And as you explained about GBR there's some checks for GBR as well in the getOrderedReportIDs. Below is a piece of code from that function:
...
reportsToDisplay.forEach((report) => {
report.displayName = ReportUtils.getReportName(report);
report.iouReportAmount = ReportUtils.getMoneyRequestSpendBreakdown(report, allReports).totalDisplaySpend;
const isPinned = report.isPinned ?? false;
const reportAction = ReportActionsUtils.getReportAction(report.parentReportID ?? '', report.parentReportActionID ?? '');
// Checking for Require Attention
if (isPinned || ReportUtils.requiresAttentionFromCurrentUser(report, reportAction)) {
pinnedAndGBRReports.push(report);
} else if (report.hasDraft) {
draftReports.push(report);
} else if (ReportUtils.isArchivedRoom(report)) {
archivedReports.push(report);
} else {
nonArchivedReports.push(report);
}
});
pinnedAndGBRReports.sort((a, b) => (a?.displayName && b?.displayName ? a.displayName.toLowerCase().localeCompare(b.displayName.toLowerCase()) : 0));
...I will further investigate the cause of high memory consumption and then update with my findings here, thanks for the pointers 👍
quinthar
commented
9 months ago Happened again; what can I do to help diagnose this?
hurali97
commented
9 months ago @quinthar I have some break through today as I was able to locate the root cause. I also tested the fix and things are looking way better now. Switching between chat list and workspaces is snappy and I didn't encounter any sort of lag OR unresponsiveness. I switched a lot of times between them.
I will post my findings here tomorrow 🤞
hurali97
commented
9 months ago Hey guys 👋
I want to share my findings on this issue with you. This issue was relatively hard to reproduce and even if I reproduced it, it was practically impossible for me to profile the memory leaks and garbage collector not able to free the memory. The reason for that is when we are profiling for memory allocations and the web page is unresponsive, everything stops at the moment, even the profiler. So there's no way forward in that area.
So I tried to profile this in chunks. To explain it, the flow this issue happens is when we are switching between chat list and workspaces. To take a chunk, we can put dummy views in chat list UI and use original view in workspaces. This gives us a chance to look for the memory issues whilst we are switching back and forth.
Following this approach I started the profile process and was able to locate a bunch of very important memory issues. Let's see them in detail:
reports object:Analysing this image from memory allocation instrument, we see that the report object still associated with detached HTML views and Garbage Collector is not able to retrieve that allocated memory. This happens in a couple of places in our profile. Exploring this further, we can see that it's bound to WorkSpacesListPage.tsx. The report object has around 15k reports in it. Take a look at the Retained Size section which means the amount of memory that will be freed once the object is removed alongside it's dependent objects. The detached HTML views takes about 2 mb of memory each and the list is relatively long.
The question is the web app became kind of usable when we placed dummy UI views in the chat list area, does it mean that the report object in other areas is also causing the issue?
The report object is supplied via withOnyx to the WorkSpacesListPage.tsx. We have report object supplied via withOnyx in 3 other places, namely ReportScreenIDSetter, SidebarLinksData and LHNOptionsList. Which means that we have more of the same problem in memory whenever we switch back and forth, hence our web app becomes slow and ultimately hangs. To explain it further let's see another issue:
withOnyx:tempState variable:
state variable:
These two variables are part of withOnyx implementation and as far as I could understand, they both hold data for the supplied key. So this means for each key we have two variables holding the same data for some implementation details reasons. We have reports object used via withOnyx in 4 prominent places, which means 8 variables holding the reports object. The point to emphasise here is that the reports object is really huge and allowing it to have multiple instances in the React Tree is not ideal. Doing so slows down our web app when we are switching between chat list and workspaces.
The ideal solution in such cases is to share the single source of truth for such huge data. To explain it, we can leverage React.Context and store the reports object there using Onyx.connect. And later where we are using withOnyx for reports we will refactor that to now use a hook for the context, say useReports(). This fixes the bottleneck we have but it also gives us the advantage of immediately mounting the component instead of having to wait to parse it through withOnyx. This is achieved because React.Context already holds the data for us and whenever we ask, it will give us immediately and no extra variables are holding the data like tempState and state.
The underlying principle of FlashList is Recycling and it doesn't play very well if our renderItems are heavy. When we are switching back and forth with all the UI in place and with above report optimisation. The web app still sometimes hangs and the main reason for that is the usage of FlashList in LHNOptionsList.
I tried to profile the memory allocations by FlashList but since web page keeps getting stuck, I couldn't profile. I also tried with chunk approach by setting dummy UI in workspaces because LHNOptionsList is in chat list area but still the web page was hanging up.
The above image is from the docs of FlashList and as we can see that the list will perform poorly if it's slow to render. If we want to use FlashList, we will need to make the renderItem fast to render but it's hard since it's associated with lots of important data. Otherwise, we can switch to FlatList on web and still keep using FlashList on mobile platforms. Alternatively, we can use FlatList now and in a separate effort we first make renderItems fast to render and then bring back FlashList.
Only case where the web page did not hang up was when I replaced FlashList with FlatList and things were back to normal. I could also profile with original UI and no hang up in that as well.
Since we can't proceed with profiling, what I will deduce from this observation is point 1. Remember the HTML detached elements were not removed by GC and reports was still part of the memory? Since our LHN is based on rendering the reports, it seems to link back to the point 1. Also, since support for FlashList is in beta on web, we can think of replacing it with FlatList.
Just a note, if we are to replace FlashList with FlatList and we know that now it works very well, we still should dedicate some effort in making the renderItems light weight.
The above three step makes our web app a lot smoother, let's see that in action below:
https://github.com/Expensify/App/assets/47336142/cd792d79-cc50-4ad1-b96e-8c1476eb0ebf
The above fixes the issue and makes our web app snappy but there are some areas that were observed during the profiling and are worth discussing over:
report object being used to access data via Onyx.connect We have Onyx connections in our util files more than 10 times. Most of them are only used for accessing the report data. In the profiling session, we see that more than 10 times the report object is holding the significant amount of memory. Above screenshot is just one example of it.
The ideal solution we can have is either Onyx.get() which helps us retrieve the updated value wherever we need it. If we can't do it for some reasons, we can create a single file with onyx connection and then read the data from it using a method, say Report.getAllReports().
I profiled the memory after closing all the connections and kept alive only a couple and the memory allocation for objects went down by margins.
JSON.Stringify in getOrderedReportIDs:We have cache mechanism in place for getOrderedReportIDs and to achieve that we generate a key through the passed arguments to the function. In the screenshot, we can see that the string key takes 25 mb at max and around ~130 mb in total . The reason is that the arguments which are used to generate the stringified key are really huge.
We should also discuss that the cache mechanism is not useful in getOrderedReportIDs. The reason is that due to such a diverse data which is prone to be non-unique because of it having a lots of keys. Which means that even if we place the cache mechanism in place, we have a limit of 5 and once that hits, we pop the oldest cache item to make room for the latest one. However, the cache is not really used because the data is different almost each time and we end up generating the key for it which is not used.
This is not only observed in this flow But in other flows as well. For example, as part of Callstack's audit, we analysed that in the App Startup, Send a message and Report screen load time, cache for getOrderedReportIDs is not used at all. In fact, it adds up to the execution time of the function by ~350 ms.
cc: @hayata-suenaga @quinthar
hayata-suenaga
commented
9 months ago @hurali97 Thank you so much for the detailed analysis and the summary of the findings. I was really impressed ❤️
I have some questions and comments on the analysis you did. I gonna share them below.
Analysing this image from memory allocation instrument, we see that the report object still associated with detached HTML views and Garbage Collector is not able to retrieve that allocated memory.
By "detached HTML views", do you mean detached components? What is the reason that the object is still associated with the detached components?
The report object is supplied via withOnyx to the WorkSpacesListPage.tsx. We have report object supplied via withOnyx in 3 other places we can leverage React.Context and store the reports object there using Onyx.connect.
I don't know why we didn't implement Onyx in a way that the data read from the Onyx storage is stored inside a single React Context. I agree that we can create a new Context for the reports object as a temporary fix. However, I believe the long time solution is to create a singular context for the Onyx data. Having multiple instances of a value under the same Onyx key seems like a waste of memory. We can consider this for future refactoring and add it to our backlog.
Otherwise, we can switch to FlatList on web and still keep using FlashList on mobile platforms.
I understand that FlatList has issues with our particular usage of it as the rendering of a list item is slow in our implementation. However, from the above comment, you seem to assume that the issue caused by the slow renderItem is only present on browsers. Could you explain why that is the case? Is that because FlashList is in beta on web?
if we are to replace FlashList with FlatList and we know that now it works very well, we still should dedicate some effort in making the renderItems light weight.
Why is the rendering of the chat item slow?
The ideal solution we can have is either Onyx.get() which helps us retrieve the updated value wherever we need it. If we can't do it for some reasons, we can create a single file with onyx connection and then read the data from it using a method, say Report.getAllReports()
Wouldn't this issue be resolved if we implement a context for reports as you mentioned earlier?
We have cache mechanism in place for
getOrderedReportIDs
You're talking about this part of the code, right? I agree with you. The cache key seems to be using the entire list of chats, which probably is unique every time. Let's remove this cache mechanism. (Even if the cache were useful sometimes, the key should have been a hashed version of the stringified parameters.)
As a general question, why is the chats object (or array?) so huge? This is the object that is used to display the list of chats (i.e. reports) on the LHN, right? Why do we have so much data inside this object when we just need to display a few pieces of information for each chat item on LHN?
mountiny
commented
9 months ago Really impressive, thank you! What would be the specific next steps which do not requite many Onyx changes in this case.
Regarding this FlashList, we have just migrated over to Flashlist due to its advantages in native side too, I think we want to going back to FlatList. We would like to focus on improving the render time
hurali97
commented
9 months ago @hayata-suenaga Thanks for the review and below I have tried to answer your queries.
By "detached HTML views", do you mean detached components? What is the reason that the object is still associated with the detached components?
Yes I mean detached components. I couldn't find the underlying reason for why the object is still associated but from what I observed is that the size of the reports object is around 15k in my case and detached components were relatively less than 15k. Which means that some of the components were safely cleared from the memory alongside the associated object but the rest of them were not cleared OR were in the process of being cleared.
However, I believe the long time solution is to create a singular context for the Onyx data.
I agree but we will have to measure the impact of this. What I am trying to say is if you have a context for onyx which has data for reports, policies and reportActions which is a huge data itself for each of them. If we connect a component to this context using useSingularOnyx() it will connect for all of the data, where we might only need let's say policies. So I think it might be better to have separate onyx context for the keys which are expected to grow. And we only connect a component to particular context from which we need the data. what do you think?
Could you explain why that is the case? Is that because FlashList is in beta on web?
I couldn't profile why FlashList's poor performance on web as I said that the web page gets stuck and the profiler also gets stuck. So I tried looking at FlashList's docs and they said that use it with caution on web as it is still in beta. Hence the reason I think that we should use FlatList on web until the support reaches stable version and then we can use FlashList again on web.
Why is the rendering of the chat item slow?
We are associating a lot of data to our renderItem. Let's see below what our renderItem looks like:
/**
* Function which renders a row in the list
*/
const renderItem = useCallback(
({item: reportID}: RenderItemProps): ReactElement => {
const itemFullReport = reports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] ?? null;
const itemReportActions = reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`] ?? null;
const itemParentReportActions = reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${itemFullReport?.parentReportID}`] ?? null;
const itemParentReportAction = itemParentReportActions?.[itemFullReport?.parentReportActionID ?? ''] ?? null;
const itemPolicy = policy?.[`${ONYXKEYS.COLLECTION.POLICY}${itemFullReport?.policyID}`] ?? null;
const transactionID = itemParentReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU ? itemParentReportAction.originalMessage.IOUTransactionID ?? '' : '';
const itemTransaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`] ?? null;
const itemComment = draftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`] ?? '';
const participants = [...ReportUtils.getParticipantsIDs(itemFullReport), itemFullReport?.ownerAccountID, itemParentReportAction?.actorAccountID].filter(Boolean) as number[];
const participantsPersonalDetails = OptionsListUtils.getPersonalDetailsForAccountIDs(participants, personalDetails);
return (
<OptionRowLHNData
reportID={reportID}
fullReport={itemFullReport}
reportActions={itemReportActions}
parentReportAction={itemParentReportAction}
policy={itemPolicy}
personalDetails={participantsPersonalDetails}
transaction={itemTransaction}
receiptTransactions={transactions}
viewMode={optionMode}
isFocused={!shouldDisableFocusOptions && reportID === currentReportID}
onSelectRow={onSelectRow}
preferredLocale={preferredLocale}
comment={itemComment}
transactionViolations={transactionViolations}
canUseViolations={canUseViolations}
onLayout={onLayoutItem}
/>
);
},
[
currentReportID,
draftComments,
onSelectRow,
optionMode,
personalDetails,
policy,
preferredLocale,
reportActions,
reports,
shouldDisableFocusOptions,
transactions,
transactionViolations,
canUseViolations,
onLayoutItem,
],
);For each renderItem we have huge data associated. The policy, reports, reportActions and personalDetails prominently.
In OptionRowLHNData we have following code for getting the linkedTransaction where we are sorting the reportActions:
const linkedTransaction = useMemo(() => {
const sortedReportActions = ReportActionsUtils.getSortedReportActionsForDisplay(reportActions);
const lastReportAction = sortedReportActions[0];
return TransactionUtils.getLinkedTransaction(lastReportAction);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [fullReport?.reportID, receiptTransactions, reportActions]);We also have some calculations for generating the item that will be rendered:
const optionItem = useMemo(() => {
// Note: ideally we'd have this as a dependent selector in onyx!
const item = SidebarUtils.getOptionData({
report: fullReport,
reportActions,
personalDetails,
preferredLocale: preferredLocale ?? CONST.LOCALES.DEFAULT,
policy,
parentReportAction,
hasViolations: !!hasViolations,
});
if (deepEqual(item, optionItemRef.current)) {
return optionItemRef.current;
}
optionItemRef.current = item;
return item;
// Listen parentReportAction to update title of thread report when parentReportAction changed
// Listen to transaction to update title of transaction report when transaction changed
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [fullReport, linkedTransaction, reportActions, personalDetails, preferredLocale, policy, parentReportAction, transaction, transactionViolations, canUseViolations]);Improving this scenario is a complicated task. Ideally, we should not be doing these calculations for each item individually and don't have to associate huge data to renderItem. We can do so by generating data in a manner that it has all of the required information before hand and we just pass it to the OptionRowLHNData without having to do further calculations.
We might need to see if such solution benefits us or otherwise by profiling but in theory, this is the most use approach.
Wouldn't this issue be resolved if we implement a context for reports as you mentioned earlier?
No. The reason is that as of my knowledge, the React.Context can't be used outside of React components and we are referring to the usage in Utils files.
As a general question, why is the chats object (or array?) so huge? This is the object that is used to display the list of chats (i.e. reports) on the LHN, right? Why do we have so much data inside this object when we just need to display a few pieces of information for each chat item on LHN?
Yes this is the object that's used to display the list of chats and why it has so much data? I can't answer you that since I am not much familiar with the internals of the codebase. From what I have observed, most of the things are dependent on each other. For example, we are filtering for archived reports, draft reports, pinned and GBR reports, sorting etc. So it's much complicated.
hurali97
commented
9 months ago @mountiny Thanks for the review.
What would be the specific next steps which do not requite many Onyx changes in this case.
From the solutions/ideas that I have shared we don't have to do changes in Onyx but rather in Expensify codebase. Like using context instead of withOnyx only for reports object. Only one change is partially related to Onyx and it's at the last of the list:
Use React.Context for reports object and replace the report object used via withOnyx in ReportScreenIDSetter, SidebarLinksData, WorkSpacesListPage and LHNOptionsList
Use React.Context for generating and holding orderedReportIds which can later be used in SidebarLinksData
Remove cache mechanism from getOrderedReportIDs
Replace FlashList with FlatList on web only
For Onyx connections in utils which are only used to retrieve data, we can have two solutions:
Regarding this FlashList, we have just migrated over to Flashlist due to its advantages in native side too, I think we want to going back to FlatList. We would like to focus on improving the render time
Yeah I agree that a lot of effort has been put in order to use FlashList and it benefits on native. That's why I think we can only use FlashList on web and still use FlatList on native platforms. Additionally, if the poor performance of FlashList is fixed with improved renderItems, we can keep using FlashList on web too.
I haven't had issues with FlashList on web in Expensify while scrolling but it only stuck when mounting/ un-mounting which happens when we switch between chat list and workspaces.
hurali97
commented
9 months ago @hayata-suenaga @mountiny One important thing I forgot to share is below and I have added it to the above specific steps.
The generation of optionItems which are used for LHNOptionsList using getOrderedReportIDs also happens each time we switch chat list and workspaces. This is not ideal and to fix that I extracted out this calculations from SidebarLinksData and put it into a React.Context and then we just use it in SidebarLinksData by calling useOrderedReportIDs(). It immediately gives us optionItems which helps us in quick mounting of SidebarLinksData.
hayata-suenaga
commented
9 months ago Ideally, we can move forward with FlatList on web only and then follow back with improved renderItem and also test it against FlashList, if it's great than bring back FlashList on web.
That's a good idea. Let's change the FlashList to FlatList for now and the come back to improve the performance of renderItem 👍
Otherwise, use a single file/ class to hold the data for reports and later use it wherever we need to retrieve latest report. ah yep we sometimes store data outside the rendering tree... and I understand we cannot use Context when we do that. Yes I agree with your approach. Let's go this route
cc @mountiny
hayata-suenaga
commented
9 months ago @hurali97 please link a PR where you're working on this.
mountiny
commented
9 months ago Yeah I agree that a lot of effort has been put in order to use FlashList and it benefits on native. That's why I think we can only use FlashList on web and still use FlatList on native platforms. Additionally, if the poor performance of FlashList is fixed with improved renderItems, we can keep using FlashList on web too.
I would probably prefer us to start with the other improvements and not change the FlashList to FlatList -> that would require larger discussion. Then we can see how the app performs with the other improvements implemented and focus on improving the renderItem first. Basically I would look at option of changing back to FlatList as the extreme case given our previous efforts to move to flashList everywhere.
hayata-suenaga
commented
9 months ago @hurali97 What is the expected time necessary to improve the performance of renderItem?
hurali97
commented
8 months ago please link a PR where you're working on this.
@hayata-suenaga The changes are currently at my local and I will prepare a PR for this tomorrow 👍
I would probably prefer us to start with the other improvements and not change the FlashList to FlatList -> that would require larger discussion.
Yes that makes sense.
What is the expected time necessary to improve the performance of renderItem?
@hayata-suenaga I am not clear on it. I will need to first devise a strategy to propose solutions for the issues I presented with renderItem. Once I am able to do that than I can share the time required to complete the effort.
@mountiny @hayata-suenaga As things stands, I will focus on the following items:
Use React.Context for reports object and replace the reports object used via withOnyx in ReportScreenIDSetter, SidebarLinksData, WorkSpacesListPage and LHNOptionsList
Use React.Context for generating and holding orderedReportIds which can later be used in SidebarLinksData
Remove cache mechanism from getOrderedReportIDs
Once the above are done, I will then focus on the following item in the same PR:
renderItem renderingAt this moment, If our web app performs as we expect, we don't need to change FlashList to FlatList. But if it doesn't and the issue is clearly with FlashList then we will discuss about it.
Also, as an additional step I will focus on:
For Onyx connections in utils which are only used to retrieve data, we can have two solutions:
This is optional and we can address it in a different PR. If we don't want to tweak Onyx, solution 2 is the way forward.
Let me know what you guys think of the steps above 🙏
hayata-suenaga
commented
8 months ago @hurali97 thank you for sharing your plan 🙇 it sounds good 👍 Let us know once you open the first PR
Also, as an additional step I will focus on:
For Onyx connections in utils which are only used to retrieve data, we can have two solutions:
Have support in onyx for get method exposed to client Otherwise, use a single file/ class to hold the data for reports and later use it wherever we need to retrieve latest report. This is optional and we can address it in a different PR. If we don't want to tweak Onyx, solution 2 is the way forward.
I also prefer the option 2 as it doesn't require modifying the Onyx codebase.
hayata-suenaga
commented
8 months ago @hurali97
I have an additional question about each report object being large. In instances where several report objects are loaded, is it possible to select only the necessary fields when subscribing to the Onyx data?
mountiny
commented
8 months ago Also, as an additional step I will focus on:
For Onyx connections in utils which are only used to retrieve data, we can have two solutions:
Have support in onyx for get method exposed to client Otherwise, use a single file/ class to hold the data for reports and later use it wherever we need to retrieve latest report.
Regarding these, I think we would need to bring such improvements to Slack first, we already had some proposals to add get function to onyx or creating a Singleton subscriber but I believe they were not accepted.
Otherwise it sounds great! @hurali97 Could you keep us posted in slack daily on progress?
hayata-suenaga
commented
8 months ago relevant discussion going on in this Slack thread
hurali97
commented
8 months ago I have an additional question about each report object being large. In instances where several report objects are loaded, is it possible to select only the necessary fields when subscribing to the Onyx data?
@hayata-suenaga Before the solution I proposed, there was selectors associated with Onyx to load only the relevant data from reports but that relevant data is not just 5-6 keys, it's more than 10 keys. Now, after the React.Context solution, I guess since it's a shared data source, there won't be any possibility to use selector 🤔 In my testing, using selector or not is the same as we still have lots of keys even after selector.
Otherwise it sounds great! @hurali97 Could you keep us posted in slack daily on progress?
@mountiny Yes sure !
hayata-suenaga
commented
8 months ago sounds good! looking forward to your updates! 🙇
zanyrenney
commented
8 months ago How are you getting on @hurali97 ? Please let us know your daily progress. Thanks!
hurali97
commented
8 months ago melvin-bot[bot]
commented
8 months ago @cubuspl42 @lschurr @hurali97 @hayata-suenaga this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
hayata-suenaga
commented
8 months ago Melvin, there are ongoing discussions in Slack threads and in the draft PR
hayata-suenaga
commented
8 months ago @hurali97 Another related performance issue was reported. I added the issue to the original post of this issue 🙇
hayata-suenaga
commented
8 months ago @hurali97 is OOO
mountiny
commented
8 months ago @kacper-mikolajczak is helping with this one while Hur is ooo
mountiny
commented
8 months ago Sorry not Kacper but @jbroma
lschurr
commented
8 months ago Any update for this one?
jbroma
commented
8 months ago @lschurr still being worked on by me, update posted today inside of #36420
melvin-bot[bot]
commented
8 months ago @cubuspl42 @lschurr @hurali97 @hayata-suenaga this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!
hayata-suenaga
commented
8 months ago Melvin, the latest discussion is here in the PR
melvin-bot[bot]
commented
8 months ago @cubuspl42, @lschurr, @hurali97, @hayata-suenaga Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
lschurr
commented
8 months ago Looks like that PR is still in draft.
hayata-suenaga
commented
8 months ago active discussion going in the draft PR
lschurr
commented
8 months ago Melvin see last comment: https://github.com/Expensify/App/issues/35704#issuecomment-1969681412
melvin-bot[bot]
commented
8 months ago @cubuspl42 @lschurr @hurali97 @hayata-suenaga this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:
Thanks!
melvin-bot[bot]
commented
8 months ago Current assignee @cubuspl42 is eligible for the Internal assigner, not assigning anyone new.
Action Performed:
Reported by David here, here, and here
This issue has two aspects:
The Chat List and Workspace List loads sometimes loads too slow
Another performance issue was reported by David -> internal Slack thread where David posted the issue
The App sometimes hangs
This issue doesn't have any defined reproduction step. David reported that he sometimes get the browser popup error saying that the page is unresponsive. Please see the screenshot below. This issue was initially reported in this GH issue.
Workaround:
N/A
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Screenshot -> https://expensify.slack.com/archives/C036QM0SLJK/p1706901348693069?thread_ts=1706887969.154719&cid=C036QM0SLJK
View all open jobs on GitHub
Upwork Automation - Do Not Edit