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

Remap indexstore contents in parallel #86

Conversation

yongjincho92
Copy link
Contributor

@yongjincho92 yongjincho92 commented Jun 12, 2023

Context
For a single indexstore that contains ~50k unit files, it can take ~25 minutes to just run index-import on.

Changes
If single indexstore is processed, process indexstore content in parallel

Tests
Tested locally with a single indexstore that has 51K index unit files.

  • Before : it took 25 minutes to run index-import.
  • After : 4 min 30 seconds to run index-import.

@yongjincho92 yongjincho92 force-pushed the yongjin/index-import/update-indexstore-in-parallel branch 3 times, most recently from 22542db to ca4f373 Compare June 15, 2023 23:40
@yongjincho92 yongjincho92 changed the title WIP If single indexstore is processed, process indexstore content in parallel Jun 15, 2023
@yongjincho92 yongjincho92 marked this pull request as ready for review June 16, 2023 00:03
@yongjincho92 yongjincho92 requested a review from keith as a code owner June 16, 2023 00:03
@keith
Copy link
Member

keith commented Jun 16, 2023

What was your end result improvement here? Also what's the issue with running this when you have more than 1 index store?

@keith
Copy link
Member

keith commented Jun 16, 2023

You'll need to fix the DCO here too. Lgtm tho

@yongjincho92
Copy link
Contributor Author

yongjincho92 commented Jun 16, 2023

What was your end result improvement here?

Tested locally with a single indexstore that has 51K index unit files.
Before : it took 25 minutes to run index-import.
After : 4 min 30 seconds to run index-import.

Also what's the issue with running this when you have more than 1 index store?

I was afraid to break that scenario, but I could try locally with multiple input indexstore as well and enable this for that scenario as well.

@keith
Copy link
Member

keith commented Jun 16, 2023

that's a huge win! I hope there's some more low hanging fruit like this too.

@yongjincho92 yongjincho92 changed the title If single indexstore is processed, process indexstore content in parallel Remap indexstore contents in parallel Jun 16, 2023
Signed-off-by: Yongjin Cho <ycho@snapchat.com>
@yongjincho92 yongjincho92 force-pushed the yongjin/index-import/update-indexstore-in-parallel branch from 812babb to f7c7815 Compare June 16, 2023 21:13
@keith keith merged commit 6558426 into MobileNativeFoundation:main Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants