-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
stream: mutable highwatermark #33346
Closed
0807Jpatel
wants to merge
9
commits into
nodejs:master
from
0807Jpatel:feat/mutable-highwatermark-objectmode
Closed
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
cf72487
feat(stream): added mutable highwatermark and object mode
0807Jpatel 134c204
fix(readme): added REPLACEME in place of version
0807Jpatel 8ab362b
fix: remove debug statements from writable stream
0807Jpatel 5ca4f8c
chore: removed mutable objectMode and highWaterMark uses setter
0807Jpatel ef07cf6
chore: removed useless function declaration and bigFile.txt
0807Jpatel e0e232f
chore: updated doc to merge getter and setter
0807Jpatel 04c4145
chore: highwatermark getter throws error in _read and _write
0807Jpatel cf28052
fix: removed unused highwatermark flags from writable stream
0807Jpatel 0b7b1ea
chore: updated debug statement
0807Jpatel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
'use strict'; | ||
|
||
const common = require('../common'); | ||
const assert = require('assert'); | ||
const stream = require('stream'); | ||
|
||
let currHighWaterMark = 20; | ||
let currObjectMode = true; | ||
let pushes = 0; | ||
const rs = new stream.Readable({ | ||
highWaterMark: currHighWaterMark, | ||
objectMode: currObjectMode, | ||
read: common.mustCall(function() { | ||
if (pushes++ === 100) { | ||
this.push(null); | ||
return; | ||
} | ||
|
||
const highWaterMark = this._readableState.highWaterMark; | ||
const objectMode = this._readableState.objectMode; | ||
|
||
// Both highwatermark and objectmode should update | ||
benjamingr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// before it makes next _read request. | ||
assert.strictEqual(highWaterMark, currHighWaterMark); | ||
assert.strictEqual(objectMode, currObjectMode); | ||
|
||
this.push(Buffer.alloc(1024)); | ||
|
||
if (pushes === 30) { | ||
this.updateReadableHighwaterMark(2048); | ||
this.updateReadableObjectMode(false); | ||
currHighWaterMark = 2048; | ||
currObjectMode = false; | ||
} | ||
|
||
}, 101) | ||
}); | ||
|
||
const ws = stream.Writable({ | ||
write: common.mustCall(function(data, enc, cb) { | ||
setImmediate(cb); | ||
}, 100) | ||
}); | ||
|
||
rs.pipe(ws); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
'use strict'; | ||
|
||
const common = require('../common'); | ||
const assert = require('assert'); | ||
const stream = require('stream'); | ||
|
||
let pushes = 0; | ||
const rs = new stream.Readable({ | ||
read: common.mustCall(function() { | ||
if (pushes++ === 100) { | ||
this.push(null); | ||
return; | ||
} | ||
|
||
this.push(Buffer.alloc(1024)); | ||
}, 101) | ||
}); | ||
|
||
let currHighWaterMark = 0; | ||
let currObjectMode = true; | ||
let read = 0; | ||
const ws = stream.Writable({ | ||
highWaterMark: currHighWaterMark, | ||
objectMode: currObjectMode, | ||
write: common.mustCall(function(data, enc, cb) { | ||
|
||
const highWaterMark = this._writableState.highWaterMark; | ||
const objectMode = this._writableState.objectMode; | ||
|
||
// Should update highwatermark and objectmode | ||
// after emptying current buffer | ||
assert.strictEqual(highWaterMark, currHighWaterMark); | ||
assert.strictEqual(objectMode, currObjectMode); | ||
|
||
if (read++ === 30) { | ||
this.updateWritableHighwaterMark(2048); | ||
this.updateWritableObjectMode(false); | ||
currHighWaterMark = 2048; | ||
currObjectMode = false; | ||
} | ||
|
||
setImmediate(cb); | ||
|
||
}, 100) | ||
}); | ||
|
||
rs.pipe(ws); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can actually see a case for this part and updating the watermarks. I even needed to use the lowWatermark at some point for a specific use case at my old job and was sad to find out we don't have one anymore :]
I think this sort of change would need to come with a concrete use case of why I'd update the watermarks.