Skip to content
This repository has been archived by the owner on Sep 4, 2023. It is now read-only.

Implement required adaptations in the extension in order to utilize the intgemm intrinsincs #75

Closed
andrenatal opened this issue Feb 4, 2022 · 5 comments · Fixed by #111
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@andrenatal
Copy link
Contributor

No description provided.

@andrenatal andrenatal added the enhancement New feature or request label Feb 4, 2022
jelmervdl added a commit to jelmervdl/translatelocally-web-ext that referenced this issue Feb 4, 2022
Because hopefully it's much faster! Also WORMHOLE or not WORMHOLE build? Which do we put into our plugin bundle? mozilla#75 and mozilla#73 might be relevant.
@abhi-agg
Copy link
Collaborator

abhi-agg commented Feb 7, 2022

We will see speed-up only if the extension is a privileged one. Otherwise, the extension will fallback to extremely slow fallback gemm.

@andrenatal
Copy link
Contributor Author

Well, if the extension is not privileged, it won't work at all, since the UI also requires privileged permissions. There's no scenario where this can work at all if not privileged.

But why can't we use the wormhole version when the intrinsics are not present, like in Release?

@abhi-agg
Copy link
Collaborator

abhi-agg commented Feb 8, 2022

But why can't we use the wormhole version when the intrinsics are not present, like in Release?

We can definitely use this approach but only in FFx because wormhole is FFx specific. For non-FFx case, we will have to use the slow fallback gemm. So the solution would be:

  1. In Firefox browser
    1. Use mozIntGemm (i.e. optimized gemm) if available else use wormhole
  2. All other browsers
    1. Use pure wasm based gemm without any wormhole

To support this workflow, the extension:

  1. Should have the capability to differentiate b/w various browser environments i.e. Firefox Nightly vs Firefox non-Nightly vs Non-Firefox). This is possible. Right?
  2. Should download the specific bergamot-translator-worker.js and bergamot-translator-worker.wasm artifacts based on the browser environment (because these artifacts are different for wormhole than the non-wormhole case) and work with them. Dynamically download Bergamot Translator Javascript artifact #82 is a prerequisite for achieving this task.

@andrenatal
Copy link
Contributor Author

andrenatal commented Feb 8, 2022

@abhi-agg the extension won't work on non-FF environments anyway, since it utilizes privileged APIs not available in other browsers, so yes, in other browsers, in the context of a regular webpage, it should Use pure wasm based gemm without any wormhole

Being specific to your question:
1 - Yes.
2 - Yes.

@andrenatal andrenatal added this to the W3 milestone Feb 10, 2022
@abhi-agg abhi-agg linked a pull request Feb 21, 2022 that will close this issue
@abhi-agg
Copy link
Collaborator

abhi-agg commented Feb 21, 2022

@abhi-agg the extension won't work on non-FF environments anyway, since it utilizes privileged APIs not available in other browsers, so yes, in other browsers, in the context of a regular webpage, it should Use pure wasm based gemm without any wormhole

This reduces the overall complexity then. Since this extension won't work on non-FFx environments, there is no need to load/import the translator artifacts that support the non-FFx browsers (i.e. non-wormhole ones) in this extension. This implies:

To support this workflow, the extension:

1. Should have the capability to differentiate b/w various browser environments i.e. Firefox Nightly vs Firefox non-Nightly vs Non-Firefox). This is possible. Right?

2. Should download the specific `bergamot-translator-worker.js` and `bergamot-translator-worker.wasm` artifacts based on the browser environment (because these artifacts are different for wormhole than the non-wormhole case) and work with them.

We don't need to do any of this because the extension should use optimized gemm whenever it is available and resorts to use wormhole-based fallback gemm otherwise. This is achievable using only 1 set of artifacts and has been fixed in #111. Hence closing this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants