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

aa async resolver #1074

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

aa async resolver #1074

wants to merge 1 commit into from

Conversation

naugtur
Copy link
Member

@naugtur naugtur commented Mar 4, 2024

Fixes #1073

Introduces a major change in the programmatic API but to a parameter that was seldom used. Will require a major bump but no consequences in adoption or such.

Performance is almost unaffected.

@github-actions github-actions bot added the pkg:@lavamoat/aa Changes in package @lavamoat/aa label Mar 4, 2024
@@ -1,7 +1,7 @@
const { realpathSync, lstatSync } = require('node:fs')
Copy link
Member Author

Choose a reason for hiding this comment

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

This was moved/changed in a different PR. I need to rebase and clean up.

* @param {(item: T) => Promise<any>} mapper
* @returns {Promise<void>}
*/
async function runParallelMax(parallelMax, items, mapper) {
Copy link
Member Author

Choose a reason for hiding this comment

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

note: this function could be much more sophisticated so that it tracks which async jobs finished and starts new ones as they do. The jobs are similar enough to each other tho, that it might not bring a lot of improvement to the execution time.

Copy link
Member Author

Choose a reason for hiding this comment

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

update - quick test of the concept:

async function runParallelMax(parallelMax, items, mapper) {
  items = items.reverse()
  return new Promise((resolve, reject) => {
    let promises = 0
    const boop = () => {
      if (items.length === 0) {
        if(promises === 0) resolve()
        return
      }
      const p = mapper(items.pop())
      promises++
      p.then(()=>{
        promises--
        boop()
      }, reject)
    }
    for (let i = 0; i < parallelMax; i++) {
      boop()
    }
  })
}

with this implementation it's a few percent slower, so there's not much to gain from smarter parallelization

BREAKING CHANGE: change in `resolver` option type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:@lavamoat/aa Changes in package @lavamoat/aa
Projects
None yet
Development

Successfully merging this pull request may close these issues.

allow async resolver in aa
1 participant