-
Notifications
You must be signed in to change notification settings - Fork 40
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
[SPIKE]: [DRGO-1289] Ramin/add isolate #348
Open
ramin-deriv
wants to merge
18
commits into
deriv-com:master
Choose a base branch
from
ramin-deriv:ramin/add_isolate
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Because it's not a sendable item
ramin-deriv
changed the title
[SPIKE]: Ramin/add isolate
[SPIKE]: [DRGO-1289] Ramin/add isolate
Oct 21, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The idea was to either move the entire WebSocket connection, including its JSON deserialization, or just the JSON deserialization to a separate isolate to make the app faster and improve its performance.
In the POC PR, we went with moving the entire connection to the isolate. However, since isolates cannot share memory with the app’s main isolate, messages have to be sent back and forth. We should note that If messages are sent and received often, this creates overhead and doesn’t improve performance. And, sending raw JSON data to the isolate for deserialization, creating Dart models there, and then sending them back to the main isolate (a two-way process for small JSONs) didn't seem to practical.
In the PR, we moved the entire WebSocket connection to the isolate. The WebSocket connection is set up there, and an instance of BinaryApi from flutter-deriv-api is created inside the isolate. This instance handles communication with the backend WebSocket server.
API requests are sent to the isolate using the message-passing system through the isolate’s port. The isolate sends these requests to the WebSocket channel. When a response is received, it creates a Dart model from the raw JSON and sends it back to the caller in the main isolate.
Setup for testing and performance comparison:
Maestro is used to create a sample automated test that opens the app, navigates to some pages, and scrolls the chart.
Flashlight is used to iterate the automation tests written by Maestro for some numbers (more tests -> more data -> more accurate results) and gives a score along with CPU, RAM usage, and average FPS.
We run these on both the current implementation and on the POC build and compare the scores and other metrics of the Flashlight report