Skip to content

Commit

Permalink
feat: Refactor selections with non-overlapping data structure (#2757)
Browse files Browse the repository at this point in the history
* fix: Add stream flag to selections that follow old select/deselect behavior

* feat: add option to create torrent with no pieces selected

* fix: fix torrent selection bug and handle deselected start

* fix: update api.md

* fix: refactor selection test toString function

* fix: libs/torrent import order

* fix: inline if statement in markUnverified

* fix: torrent.select default arguments

* fix: reversed sorting at end of torrent.select

* fix: rename selection class to plural - selections + filenames

* fix: remove @Private and @public tags from jsdoc comments

* fix: remove  redundant selections.isEmpty method

* fix: replace `self` usage with arrow function to preserve `this`

* fix: create new MinimalSelectionItem typedef and remove examples from jsdoc

* fix: use isStreamSelection from object instead separate param and fix SelectionItem typedef

* fix: add priority check to selections.remove when isStreamSelection === true

* fix: readd examples for interval comparison functions

* fix: move client-deselect test to node tests

* fix: update remove method parameter jsdoc

* fix: remove priority checks for stream selection remove

* fix: rename lastPiece to lastPieceIndex in tests

* fix: replace select priority boolean with number

* fix: handle notify functions the old way with separate notification store

* fix: issues with so and deselect flags

* fix: when reducing from by 1, handle 0 edge case (shouldn't go below 0)

* chore: simply code logic

* fix: notify not emitting for first pieces

* perf: don't create empty notifications

* fix: improve removenotification code

* fix: storing notifications the overlapping way

* fix: merge notification functions and simplify selection code

* fix: remove noop from selections notify

* fix: optional chaining for `oldNotify` call

* feat: add test for complex select-deselect behaviour to check for leaks

* fix: insideExisting handling for edge cases and add tests

---------

Co-authored-by: ThaUnknown <6506529+ThaUnknown@users.noreply.github.com>
  • Loading branch information
detarkende and ThaUnknown committed Mar 26, 2024
1 parent 625f47d commit 467f30c
Show file tree
Hide file tree
Showing 7 changed files with 501 additions and 38 deletions.
5 changes: 3 additions & 2 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ If `opts` is specified, then the default options (shown below) will be overridde
strategy: String, // Piece selection strategy, `rarest` or `sequential`(defaut=`sequential`)
noPeersIntervalTime: Number, // The amount of time (in seconds) to wait between each check of the `noPeers` event (default=30)
paused: Boolean, // If true, create the torrent in a paused state (default=false)
deselect: Boolean // If true, create the torrent with no pieces selected (default=false)
}
```

Expand Down Expand Up @@ -512,7 +513,7 @@ Selects a range of pieces to prioritize starting with `start` and ending with `e
inclusive) at the given `priority`. `notify` is an optional callback to be called when the
selection is updated with new data.

## `torrent.deselect(start, end, priority)`
## `torrent.deselect(start, end)`

Deprioritizes a range of previously selected pieces.

Expand Down Expand Up @@ -668,7 +669,7 @@ File download progress, from 0 to 1.
Selects the file to be downloaded, at the given `priority`.
Useful if you know you need the file at a later stage.

## `file.deselect([priority])`
## `file.deselect()`

Deselects the file's specific priority, which means it won't be downloaded unless someone creates a stream for it.

Expand Down
4 changes: 2 additions & 2 deletions lib/file-iterator.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export default class FileIterator extends EventEmitter {
this._missing = end - start + 1
this._criticalLength = Math.min((1024 * 1024 / this._pieceLength) | 0, 2)

this._torrent.select(this._startPiece, this._endPiece, true)
this._torrent._select(this._startPiece, this._endPiece, 1, null, true)
this.destroyed = false
}

Expand Down Expand Up @@ -88,7 +88,7 @@ export default class FileIterator extends EventEmitter {
if (this.destroyed) return
this.destroyed = true
if (!this._torrent.destroyed) {
this._torrent.deselect(this._startPiece, this._endPiece, true)
this._torrent._deselect(this._startPiece, this._endPiece, true)
}
this.emit('return')
cb(err)
Expand Down
2 changes: 1 addition & 1 deletion lib/file.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export default class File extends EventEmitter {

deselect () {
if (this.length === 0) return
this._torrent.deselect(this._startPiece, this._endPiece, false)
this._torrent.deselect(this._startPiece, this._endPiece)
}

[Symbol.asyncIterator] (opts = {}) {
Expand Down
185 changes: 185 additions & 0 deletions lib/selections.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
/**
* @typedef {Object} MinimalSelectionItem
* @property {number} from
* @property {number} to
*/

/** A selection of pieces to download.
* @typedef {MinimalSelectionItem & {
* offset: number,
* priority: number,
* notify?: function
* isStreamSelection?: boolean
* }} SelectionItem
*/

/**
* @typedef {MinimalSelectionItem & {notify: function}} NotificationItem
*/

export class Selections {
/** @type {Array<SelectionItem>} */
_items = []

/**
* @param {MinimalSelectionItem & {isStreamSelection?: boolean}} item Interval to be removed from the selection
*/
remove (item) {
for (let i = 0; i < this._items.length; i++) {
const existing = this._items[i]
if (existing.isStreamSelection) {
if (item.isStreamSelection) {
// If both are stream selections and they match, then we remove the first matching item, then we break the loop
if (existing.from === item.from && existing.to === item.to) {
this._items.splice(i, 1)
// for stream selections, we only remove one item at a time
// ergo we break the loop after removing the first matching item
break
}
} else {
// we only remove stream selections when the `isStreamSelection` flag is true and they match
continue
}
} else {
if (isLowerIntersecting(item, existing)) {
existing.to = Math.max(item.from - 1, 0)
} else if (isUpperIntersecting(item, existing)) {
existing.from = item.to + 1
} else if (isInsideExisting(item, existing)) {
const replacingItems = []
const existingStart = { ...existing, to: Math.max(item.from - 1, 0) }
if (existingStart.to - existingStart.from >= 0 && item.from !== 0) replacingItems.push(existingStart)
const existingEnd = { ...existing, from: item.to + 1 }
if (existingEnd.to - existingEnd.from >= 0) replacingItems.push(existingEnd)
this._items.splice(i, 1, ...replacingItems)
i = i - 1 + replacingItems.length // decrement i to offset splice
} else if (isCoveringExisting(item, existing)) {
this._items.splice(i, 1)
i--
}
}
}
}

/**
* @param {SelectionItem & NotificationItem} newItem
*/
insert (newItem) {
if (newItem.from > newItem.to) {
throw new Error('Invalid interval')
}
if (!newItem.isStreamSelection) {
const { notify: oldNotify } = newItem
// Merge notifications of intersecting items into the new item's notify function
const intersectingNotifyFunctions = []
for (const existing of this._items) {
if (existing.notify && isIntersecting(newItem, existing)) {
intersectingNotifyFunctions.push(existing.notify)
}
}
if (intersectingNotifyFunctions.length > 0) {
newItem.notify = () => {
intersectingNotifyFunctions.forEach(fn => fn())
oldNotify?.()
}
}
// Remove or truncate intersecting items to make room for the new item
this.remove(newItem)
}
this._items.push(newItem)
}

/** @param {(a: SelectionItem, b: SelectionItem) => number} sortFn */
sort (sortFn = (a, b) => a.from - b.from) {
this._items.sort(sortFn)
}

get length () {
return this._items.length
}

/** @param {number} index */
get (index) {
return this._items[index]
}

swap (i, j) {
const temp = this._items[i]
this._items[i] = this._items[j]
this._items[j] = temp
}

clear () {
this._items.length = 0
}

/** @returns {Generator<SelectionItem & {remove: () => void, replaceWith: (MinimalSelectionItem[]) => void}>} */
* [Symbol.iterator] () {
for (let i = 0; i < this._items.length; i++) {
const item = this._items[i]

item.remove = () => {
this._items.splice(i, 1)
i--
}
yield item
delete item.remove
}
}
}

/**
* Examples:
* existing: 1-10 | 1-10
* new_item: 8-12 | 10-15
* @param {MinimalSelectionItem} newItem
* @param {MinimalSelectionItem} existing
* @returns {boolean} returns true if the new item's lower end is intersecting with the existing item
*/
export function isLowerIntersecting (newItem, existing) {
return (newItem.from <= existing.to) && (newItem.from > existing.from) && (newItem.to > existing.to)
}

/**
* Examples:
* existing: 20-25 | 20-25
* new_item: 15-22 | 15-20
* @param {MinimalSelectionItem} newItem
* @param {MinimalSelectionItem} existing
* @returns {boolean} returns true if the new item's upper end is intersecting with the existing item
*/
export function isUpperIntersecting (newItem, existing) {
return (newItem.to >= existing.from) && (newItem.to < existing.to) && (newItem.from < existing.from)
}

/**
* Examples:
* existing: 10-20 | 10-20 | 10-20
* new_item: 12-15 | 20-20 | 15-20
* @param {MinimalSelectionItem} newItem
* @param {MinimalSelectionItem} existing
* @returns {boolean} returns true if the new item is completely inside the existing item
*/
export function isInsideExisting (newItem, existing) {
const existingIntervalSize = existing.to - existing.from
const newItemIntervalSize = newItem.to - newItem.from
return newItem.from >= existing.from && newItem.to <= existing.to && (newItemIntervalSize < existingIntervalSize)
}

/**
* Examples:
* existing: 10-20 | 10-20 | 10-20
* new_item: 10-21 | 09-20 | 10-20
* @param {MinimalSelectionItem} newItem
* @param {MinimalSelectionItem} existing
* @returns {boolean} returns true if the new item is covering the existing item
*/
export function isCoveringExisting (newItem, existing) {
return newItem.from <= existing.from && newItem.to >= existing.to
}

export const isIntersecting = (newItem, existing) => () =>
isLowerIntersecting(newItem, existing) ||
isUpperIntersecting(newItem, existing) ||
isInsideExisting(newItem, existing) ||
isCoveringExisting(newItem, existing)

0 comments on commit 467f30c

Please sign in to comment.