harfbuzz / harfbuzzjs

Providing HarfBuzz shaping library for client/server side JavaScript projects
https://harfbuzz.github.io/harfbuzzjs/
Other
204 stars 35 forks source link

Dynamically allocate memory for fonts of different sizes #39

Closed yisibl closed 2 years ago

yisibl commented 3 years ago

I have encountered this problem a long time ago, and CJK fonts such as Source Han Sans are very large. At that time, I also set the memory size to be larger(exports.memory.grow()).

https://github.com/harfbuzz/harfbuzzjs/blob/b6c12f588512cee03fb36dde3af47f3aa9274ff7/subset/test.js#L8-L10

See also: https://emscripten.org/docs/optimizing/Optimizing-Code.html#memory-growth

chearon commented 3 years ago

I had this problem with CJK fonts as well, and emoji fonts. It would be nice to have an API like:

const hbjs = await require('harfbuzzjs');
hbjs.grow(2048);

which you could use if you know you're going to be loading big fonts. I think it's reasonable for it to start out conservative like it does now.

tomasdev commented 3 years ago

Hi Caleb,

I am compiling harfbuzzjs using emscripten and would like to know more about reproduction steps to hit that memory limit.

Best,

On Mon, Jun 28, 2021, 12:36 AM Caleb Hearon @.***> wrote:

I had this problem with CJK fonts as well, and emoji fonts. It would be nice to have an API like:

const hbjs = await require('harfbuzzjs');hbjs.grow(2048);

which you could use if you know you're going to be loading big fonts. I think it's reasonable for it to start out conservative like it does now.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/harfbuzz/harfbuzzjs/issues/39#issuecomment-869233319, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACY4APFBMQ2SIDCYMV7XSDTU6RWTANCNFSM45CKFL7Q .

ebraminio commented 3 years ago

Making dynamic allocation work should/is possible, just __builtin_wasm_memory_grow/__builtin_wasm_memory_size should be used in our sbrk, https://github.com/harfbuzz/harfbuzzjs/blob/main/libc/malloc.cc#L8 which I couldn't make it work myself with some tries but should be possible.

One other solution definitely is using emscripten ofc, this project was started with emscripten actually and maybe switching to it would be better for long run and maybe we can consider switching over to it when wasi gets stable.

And please consider that the linked code is a demo code that clients will have in their code, clients already are in control of allocating memory of the wasm machine and can just allocate more memory based on size of their input, one for example easily can create a new wasm machine for each subset task based on input font size.

ebraminio commented 2 years ago

This can be considered done also as the switch to emscripten which is a great progress.

@khaledhosny I wanted to add you as a maintainer of https://www.npmjs.com/package/harfbuzzjs but I couldn't find your username in npm, it's okay to transfer full ownership of the package also just I couldn't find how to do so, alternatively you can go with another package name for the new release also if you prefer to do so.

khaledhosny commented 2 years ago

@khaledhosny I wanted to add you as a maintainer of https://www.npmjs.com/package/harfbuzzjs but I couldn't find your username in npm, it's okay to transfer full ownership of the package also just I couldn't find how to do so, alternatively you can go with another package name for the new release also if you prefer to do so.

I created an account and accepted your invitation, but I know absolutely nothing about npm, so I’m probably not going to be of much help here.

ebraminio commented 2 years ago

I created an account and accepted your invitation, but I know absolutely nothing about npm, so I’m probably not going to be of much help here.

I'll try to be helpful here, first you need to login with your account with npm login, then by npm version <update_type> or manually editing package.json you should specify the new version numbering for the package you want to publish, then you should build the project so both subset and shaping '.wasm's will be in the folder, then you should check the only needed thing will be in the final package, if something unintended exists .npmignore can be edited or you can just manually remove then, then running npm publish should be fine. Also feel free to update gh-pages branch for the demos. Thanks for your awesome work :)

ebraminio commented 2 years ago

And since https://www.npmjs.com/package/subset-font is our biggest client, guess we can ask @papandreou if is everything is ok with the publish. Thanks :)

papandreou commented 2 years ago

Thanks for the ping! I assume we're talking about a new major version? If you do a prerelease, I'd be happy to take it for a spin. I'll probably need some help if there're a lot of backwards incompatible api changes :see_no_evil:

Would be great to have the wasm heap grow dynamically. Right now subset-font wastefully allocates 125 MB for exactly this reason.

khaledhosny commented 2 years ago

I honestly don’t understand WASM memory thing, but right now it does not seem increasing memory at run time is possible at all. Emscripten supports increasing memory, but I don’t even understand how to get it working. There aren’t be any API changes in the current work AFAIK.

papandreou commented 2 years ago

@khaledhosny, I checked out the latest commit on the main branch, built hb-subset.wasm and prepared these changes: https://github.com/papandreou/subset-font/pull/18

It works with both the subset-font and subfont test suites when I npm link it all together. I also added a regression test with one of the huge fonts that previously crashed without explicitly growing the wasm heap. Overall a "go" from me 🤗

khaledhosny commented 2 years ago

I created an account and accepted your invitation, but I know absolutely nothing about npm, so I’m probably not going to be of much help here.

I'll try to be helpful here, first you need to login with your account with npm login, then by npm version <update_type> or manually editing package.json you should specify the new version numbering for the package you want to publish, then you should build the project so both subset and shaping '.wasm's will be in the folder, then you should check the only needed thing will be in the final package, if something unintended exists .npmignore can be edited or you can just manually remove then, then running npm publish should be fine. Also feel free to update gh-pages branch for the demos. Thanks for your awesome work :)

Thanks. I published 0.3.0, I hope it all went OK.

papandreou commented 2 years ago

Looks like it did! I released subset-font 1.5.0 with a dependency on harfbuzzjs@^0.3.0, and subfont 6.7.0 with a dependency on the new subset-font.

khaledhosny commented 2 years ago

Excellent!