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

doc: non-partitioned async crypto operations #17250

Closed
wants to merge 1 commit into from

Conversation

davisjam
Copy link
Contributor

@davisjam davisjam commented Nov 22, 2017

Neither crypto.randomBytes nor crypto.randomFill
partitions the work submitted to the threadpool.

This change was suggested during the discussion of #17054.
See also #17154.

Checklist
Affected core subsystem(s)

doc, crypto

Neither crypto.randomBytes nor crypto.randomFill
partitions the work submitted to the threadpool.

This change was suggested during the discussion of nodejs#17054.
See also nodejs#17154.
@nodejs-github-bot nodejs-github-bot added crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations. labels Nov 22, 2017
@davisjam
Copy link
Contributor Author

Example for crypto.randomBytes():

var nBytes = 10 * 1024*1024; /* 10 MB */
var nLongRequests = 20;
const crypto = require('crypto');

for (var i = 0; i < nLongRequests; i++) {
	crypto.randomBytes(nBytes, (buf) => {
		console.log('Long buf finished');
	});
}

crypto.randomBytes(1, (buf) => {
	console.log('Short buf finished');
});

console.log('begin');

Example for crypto.randomFill():

var nBytes = 10 * 1024*1024; /* 10 MB */
var nLongRequests = 20;
const crypto = require('crypto');

for (var i = 0; i < nLongRequests; i++) {
	var buf = Buffer.alloc(nBytes);
	crypto.randomFill(buf, () => {
		console.log('Long buf finished');
	});
}

var shortBuf = Buffer.alloc(1);
crypto.randomFill(shortBuf, () => {
	console.log('Short buf finished');
});

console.log('begin');

Both of these examples only handle the short request after the threadpool queue empties of the long ones, so "Short buf finished" prints before the final 3 "Long buf finished" messages with UV_THREADPOOL_SIZE=4.

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 27, 2017
@addaleax
Copy link
Member

Landed in 16e87ed

@addaleax addaleax closed this Nov 28, 2017
@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 28, 2017
addaleax pushed a commit that referenced this pull request Nov 28, 2017
Neither crypto.randomBytes nor crypto.randomFill
partitions the work submitted to the threadpool.

This change was suggested during the discussion of #17054.
See also #17154.

PR-URL: #17250
Refs: #17154
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
Neither crypto.randomBytes nor crypto.randomFill
partitions the work submitted to the threadpool.

This change was suggested during the discussion of #17054.
See also #17154.

PR-URL: #17250
Refs: #17154
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
Neither crypto.randomBytes nor crypto.randomFill
partitions the work submitted to the threadpool.

This change was suggested during the discussion of #17054.
See also #17154.

PR-URL: #17250
Refs: #17154
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
Neither crypto.randomBytes nor crypto.randomFill
partitions the work submitted to the threadpool.

This change was suggested during the discussion of #17054.
See also #17154.

PR-URL: #17250
Refs: #17154
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Dec 12, 2017
gibfahn pushed a commit that referenced this pull request Dec 19, 2017
Neither crypto.randomBytes nor crypto.randomFill
partitions the work submitted to the threadpool.

This change was suggested during the discussion of #17054.
See also #17154.

PR-URL: #17250
Refs: #17154
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@gibfahn
Copy link
Member

gibfahn commented Dec 19, 2017

If anyone would like to backport to 6.x please feel free (guide)!

@gibfahn gibfahn mentioned this pull request Dec 20, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
Neither crypto.randomBytes nor crypto.randomFill
partitions the work submitted to the threadpool.

This change was suggested during the discussion of #17054.
See also #17154.

PR-URL: #17250
Refs: #17154
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@gibfahn gibfahn mentioned this pull request Dec 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants