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

Try to reduce dl impact on slow computers (web) #1513

Merged
merged 2 commits into from
Sep 28, 2018
Merged

Try to reduce dl impact on slow computers (web) #1513

merged 2 commits into from
Sep 28, 2018

Conversation

Chocobozzz
Copy link
Contributor

@Chocobozzz Chocobozzz commented Sep 21, 2018

On slow computers with a fast network, web browsers may no longer respond because webtorrent is downloading chunks too fast.

The main issue is that you cannot stream a video torrent: the web browser does not have time to play the video, it is too busy downloading the video chunks.

On web browser supporting requestIdleCallback, we download chunks with a lower priority allowing the web browser to execute other tasks (like playing the video).

On slow computers with a fast network, web browsers may no longer respond because webtorrent is downloading chunks too fast.

The main issue is that you cannot stream a torrent video: the web browser does not have time to play the video, it is too busy downloading the video chunks.

On web browser supporting [requestIdleCallback](https://developer.mozilla.org/en-US/docs/Web/API/Window/requestIdleCallback#See_also), we download chunks with a lower priority allowing the web browser to execute other tasks (like playing the video).
Copy link
Member

@DiegoRBaquero DiegoRBaquero left a comment

Choose a reason for hiding this comment

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

I like this. LGTM

@DiegoRBaquero DiegoRBaquero merged commit e9b209c into webtorrent:master Sep 28, 2018
Chocobozzz added a commit to Chocobozzz/PeerTube that referenced this pull request Oct 1, 2018
webtorrent client dependency
@lock lock bot locked as resolved and limited conversation to collaborators Dec 27, 2018
@ThaUnknown
Copy link
Member

this kind of annoys me, because it's a REALLY good change, that benefits slow computers, but on the other hand it's one of the heaviest functions to run, so for fast computers some 20% of the CPU time goes to scheduling this
image

@ThaUnknown
Copy link
Member

the old code for this was:

    const ite = randomIterate(this.wires)
    let wire
    while ((wire = ite())) {
      this._updateWireWrapper(wire)
    }
  }

  _updateWireWrapper (wire) {
    const self = this

    if (typeof window !== 'undefined' && typeof window.requestIdleCallback === 'function') {
      window.requestIdleCallback(function () { self._updateWire(wire) })
    } else {
      self._updateWire(wire)
    }
  }

I'm suggesting we flip it, that requestIdleCallback is called before random iterate, then after the callback all the peers are updated, this should still help performance while not dwindling ultra-high data transfer speeds:

  /**
   * Heartbeat to update all peers and their requests.
   */
  _update () {
    if (IDLE_CALLBACK) {
      IDLE_CALLBACK(() => this._updateWireWrapper(), { timeout: 250 })
    } else {
      this._updateWireWrapper()
    }
  }

  _updateWireWrapper () {
    if (this.destroyed) return
    // update wires in random order for better request distribution
    const ite = randomIterate(this.wires)
    let wire
    while ((wire = ite())) {
      this._updateWire(wire)
    }
  }

@Chocobozzz do you think this will still accomplish its original goal? There aren't really many reproduction steps provided, and I haven't ran into the issue you're describing with this disabled, so this is kind of a blind change

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants