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

.write(Buffer) support #174

Merged
merged 7 commits into from
Oct 3, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
chore: adjust bench to show raw buffer perf
AVVS committed May 16, 2023
commit 1ed75f02a33585cc512c550280bf80d77ba9308b
37 changes: 32 additions & 5 deletions bench.js
Original file line number Diff line number Diff line change
@@ -15,11 +15,8 @@ const dummyConsole = new Console(fs.createWriteStream('/dev/null'))

const MAX = 10000

let str = ''

for (let i = 0; i < 10; i++) {
str += 'hello'
}
const buf = Buffer.alloc(50, 'hello', 'utf8')
const str = buf.toString()

setTimeout(doBench, 100)

@@ -59,6 +56,36 @@ const run = bench([
dummyConsole.log(str)
}
setImmediate(cb)
},
function benchSonicBuf (cb) {
sonic.once('drain', cb)
for (let i = 0; i < MAX; i++) {
sonic.write(buf)
}
},
function benchSonicSyncBuf (cb) {
sonicSync.once('drain', cb)
for (let i = 0; i < MAX; i++) {
sonicSync.write(buf)
}
},
function benchSonic4kBuf (cb) {
sonic4k.once('drain', cb)
for (let i = 0; i < MAX; i++) {
sonic4k.write(buf)
}
},
function benchSonicSync4kBuf (cb) {
sonicSync4k.once('drain', cb)
for (let i = 0; i < MAX; i++) {
sonicSync4k.write(buf)
}
},
function benchCoreBuf (cb) {
core.once('drain', cb)
for (let i = 0; i < MAX; i++) {
core.write(buf)
}
}
], 1000)

31 changes: 10 additions & 21 deletions index.js
Original file line number Diff line number Diff line change
@@ -93,6 +93,7 @@ function SonicBoom (opts) {
fd = fd || dest

this._bufs = []
this._lens = []
this._len = 0
this.fd = -1
this._writing = false
@@ -236,27 +237,16 @@ function emitDrain (sonic) {

inherits(SonicBoom, EventEmitter)

function bufLength (bufs) {
let idx = 0
const l = bufs.length
let size = 0
while (idx < l) {
size += bufs[idx].length
idx += 1
}
return size
}

function mergeBuf (bufs) {
function mergeBuf (bufs, len) {
if (bufs.length === 0) {
return null
return kEmptyBuffer
}

if (bufs.length === 1) {
return bufs[0]
}

return Buffer.concat(bufs)
return Buffer.concat(bufs, len)
Copy link
Member

Choose a reason for hiding this comment

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

This is the source of the slowdown. We should not merge them, but rather keep them as a list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've tried having a list here, but tests that expect single flush instead of multiples break. Will work on it further now that there are 2 separate content modes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All in all Buffer.concat seem to be cheaper than multiple fs.write as long as it doesn't have to allocate memory outside of the Buffer.poolSize. Will investigate further how that could be avoided

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tried fs.writev to avoid concat, but that made it slower than writeStream

}

SonicBoom.prototype.write = function (_data) {
@@ -267,6 +257,7 @@ SonicBoom.prototype.write = function (_data) {
const data = Buffer.isBuffer(_data) ? _data : Buffer.from(_data, 'utf8')
const len = this._len + data.length
const bufs = this._bufs
const lens = this._lens

if (this.maxLength && len > this.maxLength) {
this.emit('drop', data)
@@ -275,11 +266,13 @@ SonicBoom.prototype.write = function (_data) {

if (
bufs.length === 0 ||
bufLength(bufs[bufs.length - 1]) + data.length > this.maxWrite
lens[lens.length - 1] + data.length > this.maxWrite
) {
bufs.push([data])
lens.push(data.length)
} else {
bufs[bufs.length - 1].push(data)
lens[lens.length - 1] += data.length
}

this._len = len
@@ -393,11 +386,7 @@ SonicBoom.prototype.flushSync = function () {
let buf = kEmptyBuffer
while (this._bufs.length || buf.length) {
if (buf.length <= 0) {
buf = mergeBuf(this._bufs[0])
if (buf === null) {
this._bufs.shift()
continue
}
buf = mergeBuf(this._bufs[0], this._lens[0])
}
try {
const n = fs.writeSync(this.fd, buf)
@@ -427,7 +416,7 @@ SonicBoom.prototype.destroy = function () {
function actualWrite (sonic) {
const release = sonic.release
sonic._writing = true
sonic._writingBuf = (sonic._writingBuf.length && sonic._writingBuf) || mergeBuf(sonic._bufs.shift()) || kEmptyBuffer
sonic._writingBuf = sonic._writingBuf.length ? sonic._writingBuf : mergeBuf(sonic._bufs.shift(), sonic._lens.shift())

if (sonic.sync) {
try {