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

feat: quick TTFB & TTI for large content #138

Conversation

SgtPooki
Copy link
Member

@SgtPooki SgtPooki commented Mar 21, 2024

Title

feat: quick TTFB & TTI for large content

Description

This updates helia-service-worker-gateway to @helia/verified-fetch@1.3.0 and ensures we're respecting the request event's AbortSignal.

Summary of changes (thanks @2color):

  1. Handle aborts from original fetch event
  2. Don't await Cache API puts
  3. Respect cache-control: no-cache & pragma: no-cache

Notes & open questions

after getting #122 and ipfs/helia-verified-fetch#26 merged.. then updating @helia/verified-fetch in service-worker-gateway locally…. large video loading feels much more responsive

For the below, I hardcoded my local kubo node to ensure trustless-gateway.link was not being used.

big-buck-bunny TTFB/TTI for the code in this PR from kubo node with content:

Brave: consistently < 800ms
FireFox: 159ms

  1. A local kubo gateway configured (that has the content) npx kubo daemon
  2. cache disabled in the browser.

You can use the below process and code snippet to get TTFB:

  1. clear out service worker
  2. reload the page (you will see redirect page, make sure config is set up properly)
  3. click "load content" or reload.
  4. When the video renders, pause it.
  5. Run the below snippet in the dev console
new PerformanceObserver((entryList) => {
  const videoLoadEntries = performance.getEntriesByType('navigation')
    .filter((entry) => entry.name.includes('bafybeidsp6fva53dexzjycntiucts57ftecajcn5omzfgjx57pqfy3kwbq.ipfs.helia-sw-gateway.localhost'))

  for (const videoLoadEntry of videoLoadEntries) {
    console.log(`TTFB: ${videoLoadEntry.duration}ms for ${videoLoadEntry.name}`)
  }
}).observe({
  type: 'navigation',
  buffered: true
});

big-buck-bunny TTFB/TTI for code in this PR pulling from fresh kubo node:

Brave: 4-12s.

see #131 (comment)

big-buck-bunny TTFB/TTI for main branch code on kubo node with content

In Brave: Consistently over 1000ms.. with a fresh service worker

In Firefox: idk why but it was returning within a few ms.. old cache/sw that wouldn't go away? https://support.mozilla.org/en-US/kb/clear-cookies-and-site-data-firefox didn't always take

big-buck-bunny TTFB/TTI for code in main from fresh kubo node:

Brave: averaging ~70-90 seconds

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

@codecov-commenter
Copy link

codecov-commenter commented Mar 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (b064d20) to head (da12865).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #138   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           35        35           
  Branches         5         5           
=========================================
  Hits            35        35           
Flag Coverage Δ
node 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member Author

@SgtPooki SgtPooki left a comment

Choose a reason for hiding this comment

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

self review

@SgtPooki SgtPooki requested review from 2color and lidel March 22, 2024 02:45
@SgtPooki SgtPooki changed the title feat: quick TTFB feat: quick TTFB & TTI for large content Mar 22, 2024
@SgtPooki
Copy link
Member Author

SgtPooki commented Mar 22, 2024

changes deployed on https://bafybeidsp6fva53dexzjycntiucts57ftecajcn5omzfgjx57pqfy3kwbq.ipfs.sw.sgtpooki.com/

use https://support.mozilla.org/en-US/kb/clear-cookies-and-site-data-firefox to clear data on firefox.

TTFB on firefox from sw.sgtpooki.com is ~14000ms

src/sw.ts Outdated Show resolved Hide resolved
src/sw.ts Show resolved Hide resolved
src/sw.ts Show resolved Hide resolved
src/sw.ts Outdated Show resolved Hide resolved
Copy link
Member

@2color 2color left a comment

Choose a reason for hiding this comment

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

Barring a bunch of nits this is great!

Summary of changes (for me):

  1. Handle aborts from original fetch event
  2. Don't await Cache API puts (TIL)
  3. Respect cache-control: no-cache

(Don't cache 206s was already there but new to my eyes)

src/sw.ts Show resolved Hide resolved
@SgtPooki SgtPooki merged commit b3c08d1 into main Mar 22, 2024
23 checks passed
@SgtPooki SgtPooki deleted the 131-bug-loading-video-in-browser-native-video-player-doesnt-send-response-quick-enough branch March 22, 2024 18:19
@lidel lidel mentioned this pull request Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: loading video in browser native video player doesn't send response quick enough
3 participants