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

A way to set recommended/reasonable value for concurrency #24

Open
coreyfarrell opened this issue Sep 25, 2019 · 10 comments
Open

A way to set recommended/reasonable value for concurrency #24

coreyfarrell opened this issue Sep 25, 2019 · 10 comments

Comments

@coreyfarrell
Copy link
Contributor

Do you have a module that calculates and exports a reasonable default value for concurrency? For example what is done in sindresorhus/got#393. I'm planning to use p-map in some parts of nyc that can be async so I want to set a reasonable concurrency limit. I'd like to avoid calculating os.cpus().length multiple times and avoid duplicating the CI limiting logic.

@sindresorhus
Copy link
Owner

What's reasonable really depends on how you're using it. I don't think we can easily come up with something reasonable that works in all cases. Many use os.cpus().length, but that is wasteful if the system can actually handle more. It really depends on how heavy the task is. The point of the concurrency limit is to be able to prevent overloading the system, but in most cases the system can just handle the queuing itself.

From experience, where a concurrency limit is important is for networking, file system, and spawning child processes. For most other things, Infinity is fine and just let the system decide how much it can handle. The reason we need to set a limit at all for child processes is that the system doesn't seem to have a good way for queuing those and you can easily end up running out of descriptors.

See sindresorhus/cpy#69 (comment) for more.

// @stroncium

@coreyfarrell
Copy link
Contributor Author

In the case of NYC p-map will be reading / writing JSON files and this will be happening in the main process before and after the actual test process is spawned so it won't be competing with tests for any resources.

@stroncium
Copy link

In general concurrency limits are important for any kind of resource, including cache space, memory bandwidth, memory space, processes/threads, file descriptors, IO devices bandwidth and space and probably some more categories. There is no way to fine tune it for all the cases.

I am considering writing a lib for this, but at the moment I'm still struggling find a good way.

Problems/ideas I'm working with atm are:

  • Javascript in general, nodejs source and most of application code are so wasteful in terms of resources that some of it gets almost irrelevant. So I could run some tests which would eliminate resources it doesn't make sense to count due to overheads of other stuff. (For example as memory accesses are almost never optimized in js, counting cache doesn't help much in most cases.)
  • You can't measure much on the system. Number of cpus is in stdlib, one could probably measure amount of RAM and free disk space, number of storage devices in somewhat cross-platform manner, but I doubt one could get further than that without extreme complications.
  • For majority of cases number of cpus correlates to some extent with amount of other resources. For example it's reasonable to expect that if we got 32+ cores, the storage will have relatively high IOPS.
  • Unless the code is heavily optimized for the platform(which almost never happens in js world), there is usually relatively big range of values which have close to top performance with rapid performance falloff for values outside of this range, so we don't have to be that precise with default value.

Another thing I'm really interested in is auto tuning concurrence at runtime depending on existing performance, which might be a way forward, as it doesn't rely on gathering metrics nor predicting which ones are important and can adopt to changing workload, but got it's own downsides(might need some time to catch up to environment specifics, especially if environment changes).

As for working with files, the most effective optimizations are related to seek time in case of HDD/cache flush+fill time in case of SSD and number of descriptors. As a rule of thumb, if you're streaming big files, you want to work with 1-4 at a time on one device, if you're working with a lot of small files, most of the time you want to be at somewhere below 256 descriptors, in some cases up to 1024(and above that it's impossible to predict without knowing details). If underlying device is slow, you may be at peak performance even at 8-16, if it's fast then more in 64-128 range, but the gains from reducing concurrency too much usually are minor.
Note: there are storages with extremely high IOPS nowadays but if you work with such devices you probably know everything about that stuff already.

@stroncium
Copy link

I did a simplistic guess for trash recently.

// Educated guess, values of 16 to 64 seem to be optimal for modern SSD, 8-16 and 64-128 can be a bit slower.
// We should be ok as long as ssdCount <= cpuCount <= ssdCount*16.
// For slower media this is not as important and we rely on OS handling it for us.
const concurrency = os.cpus().length * 8;

It was for lstat then rename operations per file, the numbers are just from testing on my machine(relatively fast consumer ssd, 4 cores), but I expect the resulting values to be good enough for most real life systems.

@yxliang01
Copy link

@stroncium The library you mentioned sounds promising if it works well at the end :) (It's really complicated while have to make sure the overhead is very low, or it won't be worthy to be used...). Just wondering, auto-tuning isn't an uncommon term. Have you found any related library? Possibly, I can give some helps for the library if you would like to :D

@stroncium
Copy link

@yxliang01 I googled around a bit, but I think there is nothing around for such generic task except implementations of some algorithms(which isn't that hard once you picked algorithm anyway). If you have some libs in mind, let me know.

As for writing my own, there are still a lot of unsolved questions, so for the moment I'm just considering various approaches and modelling them.

@yxliang01
Copy link

@stroncium Right. Unfortunately, I'm not aware of such libraries at all. But, I have experience dealing with this kind of "dynamic schedulers" which are claimed to be making computers faster and more responsive(now I forgot the names already, anyways, I didn't feel much difference).

@coreyfarrell
Copy link
Contributor Author

Thanks for the responses (sorry for my slow reply). I've been trying to think of a way that p-map and similar modules could use a shared scheduler but I'm not sure this could be done safely. In theory a process-level scheduler could deal with recursive calls to p-map by not counting the first concurrent task. So:

async function processDirectory(directory) {
  await pMap(
    await fs.promises.readdir(directory),
    filename => processFile(path.join(directory, filename)),
    {scheduler}
  );
}

await pMap(directories, processDirectory, {scheduler});

If the shared scheduler were configured to allow 8 concurrent tasks in theory they could all be exhausted by calls to processDirectory but processDirectory cannot succeed unless the inner pMap is allowed to run processFile. My idea is that the scheduler would internally decrease the max concurrency by one then allow each active pMap one free task. That way if 8 calls to processDirectory were active each one would still be allowed to run 1 call to processFile without counting against concurrency. I'm not sure if this could introduce edge cases where promises can never resolve.

@yxliang01
Copy link

yxliang01 commented Dec 4, 2019

@coreyfarrell Making p-map scheduler would be one step forward than where our discussion was on. IIRC (it has been 3 months now), the problem is that we aren't aware of implementations of such scheduler. It seems hard to build one also.

@dancrew32
Copy link

dancrew32 commented Nov 3, 2023

Love this library, but our team regularly experienced issues where we could accidentally overwhelm the database if we forgot to implement the concurrency option. We regularly have to specify {concurrency: 1} or slightly higher numbers.

If any future folks end up at this issue, our solution was to implement an eslint rule that enforced the usage of the options argument with the concurrency setting. I'll leave this here just in case!

module.exports = {
  meta: {
    type: "problem",
    docs: {
      description: "pMap concurrency is Infinity by default. Please pick a reasonable concurrency, e.g. 1 or 5",
    },
    fixable: "code",
    schema: []
  },
  create: function (context) {
    return {
      CallExpression(node) {
        if (node.callee.name === 'pMap' && node.arguments.length < 3) {
          context.report({
            node,
            message: `pMap must be called with three arguments: input, mapper, and options, which must include the concurrency argument. E.g. {concurrency: 1}`,
            fix: function (fixer) {
              return fixer.insertTextAfter(node.arguments[node.arguments.length - 1], ', { concurrency: 1 }');
            }
          });
        } else if (node.callee.name === 'pMap' && node.arguments.length === 3) {
          const options = node.arguments[2];
          if (options.type !== 'ObjectExpression' || !options.properties.some(p => p.key.name === 'concurrency')) {
            context.report({
              node,
              message: 'pMap must be called with an options object that includes a concurrency property.',
              fix: function (fixer) {
                if (options.type === 'ObjectExpression' && options.properties.length === 0) {
                  return fixer.replaceText(options, '{ concurrency: 1 }');
                } else {
                  return fixer.insertTextBefore(options, '{ concurrency: 1, ...');
                }
              }
            });
          }
        }
      }
    };
  }

};

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

No branches or pull requests

5 participants