Skip to content

Commit

Permalink
feat: Implement worker threads for upload processing (#1591)
Browse files Browse the repository at this point in the history
Unfortunately the node worker threads cannot share buffers reliably with
the main thread. I learned this the hard way 😅
So I have move all of the read writes of the APKG payload to be be based
on file system access which is thread safe since we only read after
writes.

While reviewing the code path, I discovered that we actually do a lot of
wasteful reads and writes. Essentially for every APKG generation we make
2x calls due to the renaming to provide the uploader with friendly name.
I have not measured how long this takes but instead of double work we do
this one time and rely on the create_deck having the right name.

Revert of revert #1589
Reference: 2anki/create_deck#93
  • Loading branch information
aalemayhu committed Sep 1, 2024
1 parent 322120f commit 4e12767
Show file tree
Hide file tree
Showing 20 changed files with 225 additions and 313 deletions.
35 changes: 0 additions & 35 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
"get-notion-object-title": "^0.2.0",
"html-to-text": "^9.0.4",
"jsonwebtoken": "^9.0.0",
"jszip": "^3.10.1",
"knex": "^3.1.0",
"metascraper": "^5.34.7",
"metascraper-description": "^5.39.0",
Expand Down
5 changes: 4 additions & 1 deletion src/controllers/DownloadController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ class DownloadController {
return;
}

const page = DownloadPage({ id, files });
const page = DownloadPage({
id,
files: files.filter((file) => file.endsWith('.apkg')),
});
res.send(page);
});
} else {
Expand Down
63 changes: 0 additions & 63 deletions src/controllers/SimpleUploadController/SimpleUploadController.ts

This file was deleted.

16 changes: 0 additions & 16 deletions src/controllers/SimpleUploadController/createPackages.ts

This file was deleted.

24 changes: 0 additions & 24 deletions src/controllers/SimpleUploadController/createResponse.ts

This file was deleted.

4 changes: 1 addition & 3 deletions src/lib/anki/getDeckFilename.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,5 @@ test("does not append .apkg extension if it's already there", () => {
});

test("uses package name if it's available", () => {
expect(getDeckFilename(new Package('foo', Buffer.alloc(0)))).toEqual(
'foo.apkg'
);
expect(getDeckFilename(new Package('foo'))).toEqual('foo.apkg');
});
14 changes: 0 additions & 14 deletions src/lib/anki/zip.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
import JSZip from 'jszip';
import { strFromU8, unzipSync } from 'fflate';
import Package from '../parser/Package';
import { Body } from 'aws-sdk/clients/s3';
import { renderToStaticMarkup } from 'react-dom/server';
import { getUploadLimits } from '../misc/getUploadLimits';
import { isHTMLFile, isMarkdownFile } from '../storage/checks';
import getDeckFilename from './getDeckFilename';

interface File {
name: string;
Expand Down Expand Up @@ -60,17 +57,6 @@ class ZipHandler {
getFileNames() {
return this.fileNames;
}

static toZip(decks: Package[], advertisment: string | null) {
const zip = new JSZip();
for (const d of decks) {
zip.file(getDeckFilename(d), d.apkg);
}
if (advertisment) {
zip.file('README.html', advertisment);
}
return zip.generateAsync({ type: 'nodebuffer' });
}
}

export { ZipHandler, File };
47 changes: 35 additions & 12 deletions src/lib/parser/DeckParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,15 @@ import { isFileNameEqual } from '../storage/types';
import { isImageFileEmbedable, isMarkdownFile } from '../storage/checks';
import { getFileContents } from './getFileContents';
import { handleNestedBulletPointsInMarkdown } from './handleNestedBulletPointsInMarkdown';
import { checkFlashcardsLimits } from '../User/checkFlashcardsLimits';

export interface DeckParserInput {
name: string;
settings: Settings;
files: File[];
noLimits: boolean;
workspace: Workspace;
}

export class DeckParser {
globalTags: cheerio.Cheerio | null;
Expand All @@ -35,22 +44,27 @@ export class DeckParser {

files: File[];

noLimits: boolean;

public get name() {
return this.payload[0].name;
}

constructor(name: string, settings: Settings, files: File[]) {
this.settings = settings;
this.files = files || [];
this.firstDeckName = name;
constructor(input: DeckParserInput) {
this.settings = input.settings;
this.files = input.files || [];
this.firstDeckName = input.name;
this.noLimits = input.noLimits;
this.globalTags = null;

const firstFile = this.files.find((file) => isFileNameEqual(file, name));
const firstFile = this.files.find((file) =>
isFileNameEqual(file, input.name)
);

if (this.settings.nestedBulletPoints && isMarkdownFile(name)) {
if (this.settings.nestedBulletPoints && isMarkdownFile(input.name)) {
const contents = getFileContents(firstFile, false);
this.payload = handleNestedBulletPointsInMarkdown(
name,
input.name,
contents?.toString(),
this.settings.deckName,
[],
Expand All @@ -60,7 +74,7 @@ export class DeckParser {
const contents = getFileContents(firstFile, true);
this.payload = contents
? this.handleHTML(
name,
input.name,
contents.toString(),
this.settings.deckName || '',
[]
Expand Down Expand Up @@ -341,8 +355,7 @@ export class DeckParser {
return card;
}

build() {
const ws = new Workspace(true, 'fs');
build(ws: Workspace) {
const exporter = this.setupExporter(this.payload, ws.location);

for (const d of this.payload) {
Expand Down Expand Up @@ -462,9 +475,8 @@ export class DeckParser {
return exporter.save();
}

tryExperimental() {
tryExperimental(ws: Workspace) {
const fallback = new FallbackParser(this.files);
const ws = new Workspace(true, 'fs');
const exporter = this.setupExporter(this.payload, ws.location);

this.payload = fallback.run(this.settings);
Expand Down Expand Up @@ -571,6 +583,8 @@ export class DeckParser {
const parentUL = p;
const parentClass = p.attr('class') || '';

this.checkLimits(cards.length, []);

if (this.settings.toggleMode === 'open_toggle') {
dom('details').attr('open', '');
} else if (this.settings.toggleMode === 'close_toggle') {
Expand Down Expand Up @@ -651,10 +665,19 @@ export class DeckParser {

lists.forEach((list) => {
for (const child of dom(list).find('li')) {
this.checkLimits(cards.length, []);
cards.push(new Note(dom(child).html() ?? '', ''));
}
});

return cards;
}

private checkLimits(cards: number, decks: Deck[]) {
checkFlashcardsLimits({
cards: cards,
decks: decks,
paying: this.noLimits,
});
}
}
5 changes: 1 addition & 4 deletions src/lib/parser/Package.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
class Package {
name: string;

apkg: Buffer;

constructor(name: string, apkg: Buffer) {
constructor(name: string) {
this.name = name;
this.apkg = apkg;
}
}

Expand Down
Loading

0 comments on commit 4e12767

Please sign in to comment.