Skip to content
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

Run project config parse inside web worker #794

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PepijnSenders
Copy link

@PepijnSenders PepijnSenders commented Jan 17, 2023

Summary

The createProjectConfig() function is quite slow and runs sync with the application if the datafile is provided as an object.

This function could easily be run inside a web worker, freeing up the main thread, and we could use the same promise to track when it's done as we use for when we fetch the datafile.

For this PR to work, we'd still need:

  • A way to publish a worker with this project, we could parse the worker code as a string, for that we should isolate the functions it uses a bit better to reduce the load of this string. We could also have the worker as its own file in the repo, and the consumer could pass it as a parameter to the createInstance function.

@@ -197,6 +200,41 @@ export class ProjectConfigManager {
return error;
}

private async handleDatafileInWebWorker(newDatafile: string | object): Promise<OnReadyResult> {
const workerCode = '';
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be handled by some loader inside rollup; like this one: https://www.npmjs.com/package/rollup-plugin-web-worker-loader

@opti-jnguyen
Copy link
Contributor

@PepijnSenders - thanks for submitting the PR! Looked through it and looks pretty slick.

As this is technically a feature request, we'll want to document it through our feature request pipeline as well.
Here's the link to the Optimizely Feedback page where you can submit a feature request in the form of feedback: https://feedback.optimizely.com/?category=7093871273740179703

Please submit a request there when you have the chance - thanks in advance!

@PepijnSenders
Copy link
Author

Hey @opti-jnguyen, I was already discussing this change with one of your Solution Architects, do you need both? Happy to do so!

@russell-loube-optimizely

Hi @PepijnSenders , I apologize for the delayed reply here. It's my understanding that you're also speaking with one of our Solution Architects. If he has not mentioned this already, we would greatly appreciate if you could please submit this through the feature request pipeline as well.

Rest assured we are tracking the issue within our Development team. If it's not too much trouble to also submit this via the feature request pipeline, this will allow for more visibility across our Product and Engineering org.

Lastly, I believe Joshua has spoken with you about next steps for evaluating this change in your environment. I cannot comment on when we can look at this in more detail past our initial review unfortunately due to prior planned work.

Thanks for your collaboration and if you have any questions please let us know.

@mikechu-optimizely
Copy link
Contributor

This was last mentioned in internal spike/research ticket FSSDK-8510. I've requested a dev ticket be created.

@mikechu-optimizely
Copy link
Contributor

This was last mentioned in internal spike/research ticket FSSDK-8510. I've requested a dev ticket be created.

Oops. The development ticket for this is FSSDK-9614. We should be pointing it shortly.

@jame-earnin
Copy link

Any update?

@mikechu-optimizely
Copy link
Contributor

Any update?

I bumped the internal work ticket for comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants