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

Concurrent Execution with Promise.all for Enhanced Resource Utilization ✈️ #1127

Merged
merged 1 commit into from
Jul 22, 2024

Conversation

sanjaiyan-dev
Copy link
Contributor

Hello,

This PR aims to provide a minor performance boost by implementing concurrent execution of a function using Promise.all. This enhancement is expected to improve overall performance and optimize the utilization of hardware resources. By leveraging concurrent execution, we can achieve more efficient and effective utilization of system resources, resulting in improved performance.

@sanjaiyan-dev sanjaiyan-dev requested a review from a team as a code owner December 13, 2023 07:25
@jeffwidman
Copy link
Member

👋 Hey @sanjaiyan-dev thanks for stopping by and sorry for the slow review. 😢

This seems very reasonable to me, so I rebased to kickstart CI... looks like there's a lint failure and also you need to check in dist contents.

Rebuilding dist is pretty straightforward--run npm package build within a codespace (or locally as long as you've got the correct npm/node versions).

Let me know if you get stuck!

jeffwidman

This comment was marked as outdated.

Copy link
Member

@jeffwidman jeffwidman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is embarrassing, but my approval happened because I got confused by the GitHub UI... I was just trying to get CI to trigger! Probably because we're using Merge Queue's feature on this repo, and I'm still getting used to them.

Anyway, it looks like the issues I mentioned before are still occuring... can you fix the lint issue and also rebuild dist?

Once that happens, this looks good to go IMO.

Thanks!

@sanjaiyan-dev
Copy link
Contributor Author

This is embarrassing, but my approval happened because I got confused by the GitHub UI... I was just trying to get CI to trigger! Probably because we're using Merge Queue's feature on this repo, and I'm still getting used to them.

Anyway, it looks like the issues I mentioned before are still occuring... can you fix the lint issue and also rebuild dist?

Once that happens, this looks good to go IMO.

Thanks!

Hi, extremely sorry for the delay, hope I fixed the lint related issues :)

Copy link
Member

@jeffwidman jeffwidman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jeffwidman jeffwidman merged commit 5d0fcfd into github:main Jul 22, 2024
9 checks passed
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.

2 participants