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

Performance optimization of StreamSource #675

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
16 changes: 13 additions & 3 deletions lib/browser/sources/StreamSource.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,20 @@ export default class StreamSource {
return this._reader.read().then(({ value, done }) => {
if (done) {
this._done = true
} else if (this._buffer === undefined) {
this._buffer = value
} else {
this._buffer = concat(this._buffer, value)
const chunkSize = len(value)

// If all of the chunk occurs before 'start' then drop it and clear the buffer.
// This greatly improves performance when reading from a stream we haven't started processing yet and 'start' is near the end of the file.
// Rather than buffering all of the unused data in memory just to only read a chunk near the end, rather immidiately drop data which will never be read.
if (this._bufferOffset + len(this._buffer) + chunkSize < start) {
this._buffer = undefined
this._bufferOffset += chunkSize
} else if (this._buffer === undefined) {
this._buffer = value
} else {
this._buffer = concat(this._buffer, value)
}
}

return this._readUntilEnoughDataOrDone(start, end)
Expand Down
55 changes: 55 additions & 0 deletions test/spec/test-browser-specific.js
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,61 @@ describe('tus', () => {
'tus: failed to upload chunk at offset 11, caused by Error: upload was configured with a size of 100 bytes, but the source is done after 11 bytes, originated from request (method: PATCH, url: http://tus.io/uploads/foo, response code: n/a, response text: n/a, request id: n/a)',
)
})

it('should upload data correctly when using a non zero starting offset', async () => {
const reader = makeReader('hello world', 1)

const testStack = new TestHttpStack()
const options = {
httpStack: testStack,
uploadUrl: "http://tus.io/uploads/fileid",
retryDelays: [],
chunkSize: 6,
uploadLengthDeferred: true,
onSuccess() {}
}

const upload = new tus.Upload(reader, options)
upload.start()

let req = await testStack.nextRequest()
expect(req.url).toBe('http://tus.io/uploads/fileid')
expect(req.method).toBe('HEAD')
expect(req.requestHeaders['Tus-Resumable']).toBe('1.0.0')

// Respond with a non zero offset to test that the stream that is created for the reader returns the correct data and ignores the data in the stream before the offset.
req.respondWith({
status: 200,
responseHeaders: {
"Upload-Offset": 6,
},
})

req = await testStack.nextRequest()
expect(req.url).toBe('http://tus.io/uploads/fileid')
expect(req.method).toBe('PATCH')
expect(req.requestHeaders['Upload-Offset']).toBe(6)
expect(req.requestHeaders['Content-Type']).toBe('application/offset+octet-stream')
expect(req.body.length).toBe(5)
expect(req.body).toEqual(["w","o","r","l", "d"])

req.respondWith({
status: 204,
responseHeaders: {
'Upload-Offset': 11,
},
})

req = await testStack.nextRequest()

expect(req.url).toBe('http://tus.io/uploads/fileid')
expect(req.method).toBe('PATCH')
expect(req.requestHeaders['Upload-Offset']).toBe(11)
expect(req.requestHeaders['Content-Type']).toBe('application/offset+octet-stream')
expect(req.body).toBe(null)

await options.onSuccess.toBeCalled
})
})

describe('resolving of URIs', () => {
Expand Down