From 53431aec392dd8b8739845e4c3f78acd96a5a8f2 Mon Sep 17 00:00:00 2001 From: Yusuke Sakurai Date: Sun, 25 Apr 2021 05:05:46 +0900 Subject: [PATCH 1/7] fix(io): readDelim returns wrong result if delim strides over chunks --- io/bufio.ts | 9 ++--- io/bufio_test.ts | 91 ++++++++++++++++++++++++++++++++---------------- 2 files changed, 64 insertions(+), 36 deletions(-) diff --git a/io/bufio.ts b/io/bufio.ts index d4d13ebf6c48..92d898431eec 100644 --- a/io/bufio.ts +++ b/io/bufio.ts @@ -641,7 +641,7 @@ export async function* readDelim( // Modified KMP let inspectIndex = 0; let matchIndex = 0; - let itr = chunks.iterator(); + let itr: IterableIterator; let curr = -1; while (true) { const inspectArr = new Uint8Array(bufSize); @@ -655,9 +655,8 @@ export async function* readDelim( return; } chunks.add(inspectArr, 0, result); - if (inspectIndex === 0 && chunks.size() > 0) { - curr = itr.next().value; - } + itr = chunks.iterator(inspectIndex); + curr = itr.next().value; while (inspectIndex < chunks.size()) { if (curr === delim[matchIndex]) { curr = itr.next().value; @@ -672,8 +671,6 @@ export async function* readDelim( chunks.shift(inspectIndex); inspectIndex = 0; matchIndex = 0; - itr = chunks.iterator(); - curr = itr.next().value; } } else { if (matchIndex === 0) { diff --git a/io/bufio_test.ts b/io/bufio_test.ts index 927ca09bf65f..062da5722aee 100644 --- a/io/bufio_test.ts +++ b/io/bufio_test.ts @@ -178,15 +178,6 @@ Deno.test("bufioReadString", async function () { } }); -const testInput = encoder.encode( - "012\n345\n678\n9ab\ncde\nfgh\nijk\nlmn\nopq\nrst\nuvw\nxy", -); -const testInputrn = encoder.encode( - "012\r\n345\r\n678\r\n9ab\r\ncde\r\nfgh\r\nijk\r\nlmn\r\nopq\r\nrst\r\n" + - "uvw\r\nxy\r\n\n\r\n", -); -const testOutput = encoder.encode("0123456789abcdefghijklmnopqrstuvwxy"); - // TestReader wraps a Uint8Array and returns reads of a specific length. class TestReader implements Deno.Reader { constructor(private data: Uint8Array, private stride: number) {} @@ -207,6 +198,14 @@ class TestReader implements Deno.Reader { return Promise.resolve(nread); } } +const testInput = encoder.encode( + "012\n345\n678\n9ab\ncde\nfgh\nijk\nlmn\nopq\nrst\nuvw\nxy", +); +const testInputrn = encoder.encode( + "012\r\n345\r\n678\r\n9ab\r\ncde\r\nfgh\r\nijk\r\nlmn\r\nopq\r\nrst\r\n" + + "uvw\r\nxy\r\n\n\r\n", +); +const testOutput = encoder.encode("0123456789abcdefghijklmnopqrstuvwxy"); async function testReadLine(input: Uint8Array) { for (let stride = 1; stride < 2; stride++) { @@ -243,6 +242,41 @@ Deno.test("bufioReadLine", async function () { await testReadLine(testInputrn); }); +Deno.test("[io] readStringDelim basic", async () => { + const delim = "!#$%&()=~"; + const exp = [ + "", + "a", + "bc", + "def", + "", + "!", + "!#", + "!#$%&()=", + "#$%&()=~", + "", + "", + ]; + const str = exp.join(delim); + const arr: string[] = []; + for await (const v of readStringDelim(new StringReader(str), delim)) { + arr.push(v); + } + assertEquals(arr, exp); +}); + +Deno.test("[io] readStringDelim bigger delim than buf size", async () => { + // 0123456789... + const delim = Array.from({ length: 1025 }).map((v, i) => i % 10).join(""); + const exp = ["", "a", "bc", "def", "01", "012345678", "123456789", "", ""]; + const str = exp.join(delim); + const arr: string[] = []; + for await (const v of readStringDelim(new StringReader(str), delim)) { + arr.push(v); + } + assertEquals(arr, exp); +}); + Deno.test("bufioPeek", async function () { const decoder = new TextDecoder(); const p = new Uint8Array(10); @@ -459,27 +493,24 @@ Deno.test("readStringDelimAndLines", async function () { assertEquals(lines_, ["0", "1", "2", "3", "4", "5", "6", "7", "8", "9"]); }); -Deno.test( - "bufReaderShouldNotShareArrayBufferAcrossReads", - async function () { - const decoder = new TextDecoder(); - const data = "abcdefghijklmnopqrstuvwxyz"; - const bufSize = 25; - const b = new BufReader(new StringReader(data), bufSize); - - const r1 = (await b.readLine()) as ReadLineResult; - assert(r1 !== null); - assertEquals(decoder.decode(r1.line), "abcdefghijklmnopqrstuvwxy"); - - const r2 = (await b.readLine()) as ReadLineResult; - assert(r2 !== null); - assertEquals(decoder.decode(r2.line), "z"); - assert( - r1.line.buffer !== r2.line.buffer, - "array buffer should not be shared across reads", - ); - }, -); +Deno.test("bufReaderShouldNotShareArrayBufferAcrossReads", async function () { + const decoder = new TextDecoder(); + const data = "abcdefghijklmnopqrstuvwxyz"; + const bufSize = 25; + const b = new BufReader(new StringReader(data), bufSize); + + const r1 = (await b.readLine()) as ReadLineResult; + assert(r1 !== null); + assertEquals(decoder.decode(r1.line), "abcdefghijklmnopqrstuvwxy"); + + const r2 = (await b.readLine()) as ReadLineResult; + assert(r2 !== null); + assertEquals(decoder.decode(r2.line), "z"); + assert( + r1.line.buffer !== r2.line.buffer, + "array buffer should not be shared across reads", + ); +}); Deno.test({ name: "Reset buffer after flush", From e5338680a59e835e40189017e5200defaa33a517 Mon Sep 17 00:00:00 2001 From: Yusuke Sakurai Date: Sun, 25 Apr 2021 05:13:23 +0900 Subject: [PATCH 2/7] revert unrelated changes --- io/bufio_test.ts | 56 ++++++++++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/io/bufio_test.ts b/io/bufio_test.ts index 062da5722aee..ee5d930f3f54 100644 --- a/io/bufio_test.ts +++ b/io/bufio_test.ts @@ -178,6 +178,15 @@ Deno.test("bufioReadString", async function () { } }); +const testInput = encoder.encode( + "012\n345\n678\n9ab\ncde\nfgh\nijk\nlmn\nopq\nrst\nuvw\nxy", +); +const testInputrn = encoder.encode( + "012\r\n345\r\n678\r\n9ab\r\ncde\r\nfgh\r\nijk\r\nlmn\r\nopq\r\nrst\r\n" + + "uvw\r\nxy\r\n\n\r\n", +); +const testOutput = encoder.encode("0123456789abcdefghijklmnopqrstuvwxy"); + // TestReader wraps a Uint8Array and returns reads of a specific length. class TestReader implements Deno.Reader { constructor(private data: Uint8Array, private stride: number) {} @@ -198,14 +207,6 @@ class TestReader implements Deno.Reader { return Promise.resolve(nread); } } -const testInput = encoder.encode( - "012\n345\n678\n9ab\ncde\nfgh\nijk\nlmn\nopq\nrst\nuvw\nxy", -); -const testInputrn = encoder.encode( - "012\r\n345\r\n678\r\n9ab\r\ncde\r\nfgh\r\nijk\r\nlmn\r\nopq\r\nrst\r\n" + - "uvw\r\nxy\r\n\n\r\n", -); -const testOutput = encoder.encode("0123456789abcdefghijklmnopqrstuvwxy"); async function testReadLine(input: Uint8Array) { for (let stride = 1; stride < 2; stride++) { @@ -493,24 +494,27 @@ Deno.test("readStringDelimAndLines", async function () { assertEquals(lines_, ["0", "1", "2", "3", "4", "5", "6", "7", "8", "9"]); }); -Deno.test("bufReaderShouldNotShareArrayBufferAcrossReads", async function () { - const decoder = new TextDecoder(); - const data = "abcdefghijklmnopqrstuvwxyz"; - const bufSize = 25; - const b = new BufReader(new StringReader(data), bufSize); - - const r1 = (await b.readLine()) as ReadLineResult; - assert(r1 !== null); - assertEquals(decoder.decode(r1.line), "abcdefghijklmnopqrstuvwxy"); - - const r2 = (await b.readLine()) as ReadLineResult; - assert(r2 !== null); - assertEquals(decoder.decode(r2.line), "z"); - assert( - r1.line.buffer !== r2.line.buffer, - "array buffer should not be shared across reads", - ); -}); +Deno.test( + "bufReaderShouldNotShareArrayBufferAcrossReads", + async function () { + const decoder = new TextDecoder(); + const data = "abcdefghijklmnopqrstuvwxyz"; + const bufSize = 25; + const b = new BufReader(new StringReader(data), bufSize); + + const r1 = (await b.readLine()) as ReadLineResult; + assert(r1 !== null); + assertEquals(decoder.decode(r1.line), "abcdefghijklmnopqrstuvwxy"); + + const r2 = (await b.readLine()) as ReadLineResult; + assert(r2 !== null); + assertEquals(decoder.decode(r2.line), "z"); + assert( + r1.line.buffer !== r2.line.buffer, + "array buffer should not be shared across reads", + ); + }, +); Deno.test({ name: "Reset buffer after flush", From 2babab3ac87a27582da04709dc4bff8ad33b7d01 Mon Sep 17 00:00:00 2001 From: Yusuke Sakurai Date: Sun, 25 Apr 2021 05:14:13 +0900 Subject: [PATCH 3/7] Update bufio_test.ts --- io/bufio_test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/io/bufio_test.ts b/io/bufio_test.ts index ee5d930f3f54..6012c66c892a 100644 --- a/io/bufio_test.ts +++ b/io/bufio_test.ts @@ -268,7 +268,7 @@ Deno.test("[io] readStringDelim basic", async () => { Deno.test("[io] readStringDelim bigger delim than buf size", async () => { // 0123456789... - const delim = Array.from({ length: 1025 }).map((v, i) => i % 10).join(""); + const delim = Array.from({ length: 1025 }).map((_, i) => i % 10).join(""); const exp = ["", "a", "bc", "def", "01", "012345678", "123456789", "", ""]; const str = exp.join(delim); const arr: string[] = []; From c0bb768b51609425f1a20349dcb2f401f0631e28 Mon Sep 17 00:00:00 2001 From: Yusuke Sakurai Date: Sun, 25 Apr 2021 05:16:02 +0900 Subject: [PATCH 4/7] Update bufio.ts --- io/bufio.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/io/bufio.ts b/io/bufio.ts index 92d898431eec..c4f5f61a4ec4 100644 --- a/io/bufio.ts +++ b/io/bufio.ts @@ -641,8 +641,6 @@ export async function* readDelim( // Modified KMP let inspectIndex = 0; let matchIndex = 0; - let itr: IterableIterator; - let curr = -1; while (true) { const inspectArr = new Uint8Array(bufSize); const result = await reader.read(inspectArr); @@ -655,8 +653,8 @@ export async function* readDelim( return; } chunks.add(inspectArr, 0, result); - itr = chunks.iterator(inspectIndex); - curr = itr.next().value; + const itr = chunks.iterator(inspectIndex); + let curr = itr.next().value; while (inspectIndex < chunks.size()) { if (curr === delim[matchIndex]) { curr = itr.next().value; From 9ffd193660bc8d9840f8e8f783145ab0f2246804 Mon Sep 17 00:00:00 2001 From: Yusuke Sakurai Date: Sun, 25 Apr 2021 05:27:33 +0900 Subject: [PATCH 5/7] got faster --- io/bufio.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/io/bufio.ts b/io/bufio.ts index c4f5f61a4ec4..64652badd9c5 100644 --- a/io/bufio.ts +++ b/io/bufio.ts @@ -653,12 +653,11 @@ export async function* readDelim( return; } chunks.add(inspectArr, 0, result); - const itr = chunks.iterator(inspectIndex); - let curr = itr.next().value; + let localIndex = 0; while (inspectIndex < chunks.size()) { - if (curr === delim[matchIndex]) { - curr = itr.next().value; + if (inspectArr[localIndex] === delim[matchIndex]) { inspectIndex++; + localIndex++; matchIndex++; if (matchIndex === delimLen) { // Full match @@ -673,7 +672,7 @@ export async function* readDelim( } else { if (matchIndex === 0) { inspectIndex++; - curr = itr.next().value; + localIndex++; } else { matchIndex = delimLPS[matchIndex - 1]; } From 82b327a6b19aa21678fd7f79e7c537b586a00ca8 Mon Sep 17 00:00:00 2001 From: Yusuke Sakurai Date: Mon, 26 Apr 2021 06:31:18 +0900 Subject: [PATCH 6/7] fix: createLPS bug --- io/bufio.ts | 2 +- io/bufio_test.ts | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/io/bufio.ts b/io/bufio.ts index 64652badd9c5..e47023029d13 100644 --- a/io/bufio.ts +++ b/io/bufio.ts @@ -621,7 +621,7 @@ function createLPS(pat: Uint8Array): Uint8Array { lps[i] = 0; i++; } else { - prefixEnd = pat[prefixEnd - 1]; + prefixEnd = lps[prefixEnd - 1]; } } return lps; diff --git a/io/bufio_test.ts b/io/bufio_test.ts index 6012c66c892a..e2d057ba9173 100644 --- a/io/bufio_test.ts +++ b/io/bufio_test.ts @@ -278,6 +278,17 @@ Deno.test("[io] readStringDelim bigger delim than buf size", async () => { assertEquals(arr, exp); }); +Deno.test("[io] readStringDelim delim=1213", async () => { + const delim = "1213"; + const exp = ["", "a", "bc", "def", "01", "012345678", "123456789", "", ""]; + const str = exp.join(delim); + const arr: string[] = []; + for await (const v of readStringDelim(new StringReader(str), "1213")) { + arr.push(v); + } + assertEquals(arr, exp); +}); + Deno.test("bufioPeek", async function () { const decoder = new TextDecoder(); const p = new Uint8Array(10); From 66d91e8d18844e417caa5063df643c2de46e6aff Mon Sep 17 00:00:00 2001 From: Yusuke Sakurai Date: Mon, 26 Apr 2021 06:32:28 +0900 Subject: [PATCH 7/7] trigger