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

perf(DASH): reduce looping and remove chaining awaits in period #5774

Merged

Conversation

vanyaxk
Copy link
Contributor

@vanyaxk vanyaxk commented Oct 16, 2023

This change removes chaining awaits in period combiner when combining variants and parses streams in one loop instead of 4

@avelad avelad changed the title perf(periods): reduce looping and remove chaining awaits perf(DASH): reduce looping and remove chaining awaits in period Oct 16, 2023
@avelad avelad added type: performance A performance issue priority: P1 Big impact or workaround impractical; resolve before feature release component: DASH The issue involves the MPEG DASH manifest format labels Oct 16, 2023
@avelad avelad added this to the v4.6 milestone Oct 16, 2023
@shaka-bot
Copy link
Collaborator

Incremental code coverage: 100.00%

@theodab theodab merged commit be755e8 into shaka-project:main Oct 18, 2023
22 checks passed
Robloche pushed a commit to Robloche/shaka-player that referenced this pull request Nov 30, 2023
…a-project#5774)

This change removes chaining awaits in period combiner when combining variants and parses streams in one loop instead of 4

Co-authored-by: Ivan Kohut <ivan.kohut@lamin.ar>
gkatsev pushed a commit to sky-hugolima/shaka-player-contrib that referenced this pull request Dec 6, 2023
* feat: Improve performance of multi-period DASH parsing (shaka-project#5350)

For manifests with many streams, we saw the bulk of the manifest parse
time going towards period combiner. Specifically: splitting codecs and
comparing DRMs. This change cuts our manifest parse time for large
manifests by nearly 30%.

There are two changes:

1. Memoize the normalized codecs inside the period combiner.
2. Short circuit the DRM compatibility check if the DRMInfos arrays are
the same object. We run a custom manifest parser and are therefore able
to re-use the same drm infos array for functionally equivalent drms, but
short circuiting also helps since in the case that the upstream period
combiner is comparing a candidate stream with itself.

* perf(DASH): reduce looping and remove chaining awaits in period (shaka-project#5774)

This change removes chaining awaits in period combiner when combining variants and parses streams in one loop instead of 4

Co-authored-by: Ivan Kohut <ivan.kohut@lamin.ar>

* perf(DASH): PeriodCombiner optimisations (shaka-project#5837)

- reduce number of iterations during filtering out stream duplicates
- reduce number of iterations when extending output stream
- for audio or video only content, reuse existing stream array instead
of copying it

* some low hanging fruits optimized

* match optimization by looking for duplicates

* use stream once

* add hash map and config

* concat arrays

* use slice() instead of Array.from() to clone

* remove manifest.filterOutStreamDuplicates

As we're using Map for making unique keys already, we don't need to
filter out duplicates anymore.

* simplify generating key

* alias for shaka.util.PeriodCombiner

---------

Co-authored-by: baconz <baconz@users.noreply.github.com>
Co-authored-by: Ivan <48260572+vanyaxk@users.noreply.github.com>
Co-authored-by: Ivan Kohut <ivan.kohut@lamin.ar>
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Dec 17, 2023
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Dec 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component: DASH The issue involves the MPEG DASH manifest format priority: P1 Big impact or workaround impractical; resolve before feature release status: archived Archived and locked; will not be updated type: performance A performance issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants