-
-
Notifications
You must be signed in to change notification settings - Fork 26
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
refactor(export): use createWriteStream to export json #277
Conversation
Pull Request Test Coverage Report for Build 13085724941Details
💛 - Coveralls |
src/database.ts
Outdated
let prefix = ''; | ||
if (i) { | ||
prefix = ','; | ||
} | ||
await handle.write(`${prefix}"${key}":${models[key]._export()}`); |
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.
Since we are targeting the modern Node.js versions now, maybe we could try fs.createWriteStream
?
let p: Promise<unknown> | undefined;
const writeStream = fs.createWriteStream(filePath);
p = asyncWriteToStream(writeStream, prefix + '\n');
if (p) await p;
for (let i = 0; i < itemsLength; i++) {
p = asyncWriteToStream(writeStream, items[i] + '\n');
// eslint-disable-next-line no-await-in-loop -- stream high water mark
if (p) await p;
}
// You can find `asyncWriteToStream` implementation at https://github.com/SukkaW/foxts/blob/a7f4e40434b1a46d58d5062b9df7713486f677b2/src/async-write-to-stream/index.ts#L17
… refactor/exports
const raw = data[keys[i]]; | ||
if (raw) { | ||
const prefix = i === 0 ? '' : ','; | ||
p = asyncWriteToStream(writeStream, `${prefix}${JSON.stringify(schema._exportDatabase(cloneDeep(raw) as object))}`); |
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.
This new function writes each cloned object directly to stream, making it more friendly for escape analysis
Co-authored-by: Sukka <[email protected]> Signed-off-by: D-Sketon <[email protected]>
I wonder why all the tests on windows are failing :( |
fixed |
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.
LGTM. I wonder about the new implementation's performance though.
@SukkaW Cold Processing
Hot Processing
Another Cold Processing
4x hexo-many-posts Cold Processing
Hot Processing
Another Cold Processing
8x hexo-many-posts Cold Processing
Hot Processing
Another Cold Processing
12x hexo-many-posts Cold Processing
Hot Processing
Another Cold Processing
|
BTW, I realised that coverage was not working, so I locked mocha to version 11.0.2 |
This PR solves part of the situation in hexojs/hexo#4922 (it's still possible to throw an exception if the word count of a post is too long) |
check list
Description
I'm wondering why you don't just splice the strings when exporting? It's faster and takes less memory.




before
You can see the memory rise at the end of
hexo g
.after
Additional information