[PROPOSAL] DASH: Remove "native" JS parser #1482
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
peaBerberian
added
DASH
peaBerberian
force-pushed
the
misc/remove-dash-native
branch
3 times, most recently
from
f5cf0c6 to
4413c98
Compare
peaBerberian
force-pushed
the
misc/remove-dash-native
branch
from
4413c98 to
bb4a256
Compare
peaBerberian
force-pushed
the
misc/remove-dash-native
branch
3 times, most recently
from
44c2921 to
13bf0e5
Compare
peaBerberian
force-pushed
the
misc/remove-dash-native
branch
from
13bf0e5 to
046a368
Compare
peaBerberian
force-pushed
the
misc/remove-dash-native
branch
from
046a368 to
b39f2dd
Compare
peaBerberian
force-pushed
the
misc/remove-dash-native
branch
from
b39f2dd to
731e22d
Compare
|
Automated performance checks have been performed on commit Tests results✅ Tests have passed. Performance tests 1st run outputNo significative change in performance for tests:
If you want to skip performance checks for latter commits, add the |
From the v4.1.0, we had 3 MPD parsers:
- The "native" one, relying on the web's DOMParser API.
This is historically the first one we used and still the one we rely
by default on main thread.
- The "WebAssembly" one, added in 2021.
This one was initially written when we had to manage multi-megabytes
MPD. Those took forever to parse on low-end devices but worse even
on a PC, parsing it often (e.g. for a live content) could lead to a
crash after several hours due to GC pressure.
The idea was to parse the MPD in another thread (the initial
MULTI_THREAD attempts date back from here :p) and to rely on
WebAssembly to better control memory usage and performance (also the
"native" one wasn't usable anyway on a WebWorker due to browser
limitations).
It turned out that the WebAssembly version was so light (note: we
also rely on XML StAX parsing instead of DOM parsing which may have
helped for that part) and fast that we didn't yet need the
complexity of bringing another thread here.
- The "fast js" one, added at the last `v4.1.0` release.
This one follows attempts to make the `MULTI_THREAD` feature usable
on non-WebAssembly devices. We noticed that other developers had
recently made attempts for fast JS parsing without even needing the
use of the DOMParser. They reported even faster performance due to
much fewer XML integrity checks (which is OK for us, as MPD parsing
performance is one of the most important aspect for us).
This commit proposes that we remove the "native" one to just replace it
by the "fast js" one.
The "fast js" one has already been used in production for more than 6
months without issue, is equal-to-faster than the native one and it
would lead to much less code.
peaBerberian
force-pushed
the
misc/remove-dash-native
branch
from
731e22d to
b32084c
Compare
|
Automated performance checks have been performed on commit Tests results✅ Tests have passed. Performance tests 1st run outputNo significative change in performance for tests:
If you want to skip performance checks for latter commits, add the |
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.
From the v4.1.0, we had 3 MPD parsers:
The "native" one, relying on the web's DOMParser API.
This is historically the first one we used and still the one we rely by default on main thread.
The "WebAssembly" one, added in 2021.
This one was initially written when we had to manage multi-megabytes MPD. Those took forever to parse on low-end devices but worse even on a PC, parsing it often (e.g. for a live content) could lead to a crash after several hours due to GC pressure.
The idea was to parse the MPD in another thread (the initial MULTI_THREAD attempts date back from here :p) and to rely on WebAssembly to better control memory usage and performance (also the "native" one wasn't usable anyway on a WebWorker due to browser limitations).
It turned out that the WebAssembly version was so light (note: we also rely on XML StAX parsing instead of DOM parsing which may have helped for that part) and fast that we didn't yet need the complexity of bringing another thread here.
The "fast js" one, added at the last
v4.1.0release.This one follows attempts to make the
MULTI_THREADfeature usable on non-WebAssembly devices. We noticed that other developers had recently made attempts for fast JS parsing without even needing the use of the DOMParser. They reported even faster performance due to much fewer XML integrity checks (which is OK for us, as MPD parsing performance is one of the most important aspect for us).This PR proposes that we remove the "native" one to just replace it by the "fast js" one.
The "fast js" one has already been used in production for more than 6 months without issue, is equal-to-faster than the native one and it would lead to much less code.