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: PeriodCombiner improvements #6005

Merged
merged 7 commits into from
Jan 9, 2024

Conversation

tykus160
Copy link
Member

@tykus160 tykus160 commented Dec 8, 2023

Summary of changes:

  • create dummy streams during preparing arrays of input streams
  • use hash map to quickly find perfect stream matches across periods
  • hash map also automatically removes duplicates, so remove previous logic for finding them
  • check earlier are we trying to create output stream from dummy stream
  • 2 changes from above also gives us a possibility to simplify areCompatible() & isBetterMatch() methods
  • reduce creation of spare collections when concatenating streams
  • reduce conditional logic when possible

I was testing PeriodCombiner.combinePeriods() performance of mentioned changes on Tizen 2021 on 2 streams and I've got following results:

content upstream proposed changes
stream 1 23 ms 17 ms
stream 2 484 ms 191 ms

Both streams are VOD.
Stream 1 has 18 periods with 6 video & audio tracks in each.
Stream 2 has 18 periods with 6 video tracks & 36 audio tracks in each.

@shaka-bot
Copy link
Collaborator

Incremental code coverage: 96.20%

@avelad avelad added type: performance A performance issue priority: P1 Big impact or workaround impractical; resolve before feature release labels Dec 23, 2023
@avelad avelad added this to the v5.0 milestone Dec 23, 2023
@avelad avelad requested review from avelad and theodab January 8, 2024 08:34
@avelad avelad merged commit 4022788 into shaka-project:main Jan 9, 2024
20 of 21 checks passed
avelad pushed a commit that referenced this pull request Jan 9, 2024
Summary of changes:
- create dummy streams during preparing arrays of input streams
- use hash map to quickly find perfect stream matches across periods
- hash map also automatically removes duplicates, so remove previous
logic for finding them
- check earlier are we trying to create output stream from dummy stream
- 2 changes from above also gives us a possibility to simplify
`areCompatible()` & `isBetterMatch()` methods
- reduce creation of spare collections when concatenating streams
- reduce conditional logic when possible

I was testing `PeriodCombiner.combinePeriods()` performance of mentioned
changes on Tizen 2021 on 2 streams and I've got following results:

| content | upstream | proposed changes |
| - | -: | -: |
| stream 1 | 23 ms | 17 ms |
| stream 2 | 484 ms | 191 ms |

Both streams are VOD.
Stream 1 has 18 periods with 6 video & audio tracks in each.
Stream 2 has 18 periods with 6 video tracks & 36 audio tracks in each.
@tykus160 tykus160 deleted the wt-period-combiner-upstream branch January 9, 2024 12:19
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Mar 9, 2024
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Mar 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
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.

3 participants