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

use faster timers #1908

Merged
merged 16 commits into from
Feb 5, 2023
7 changes: 4 additions & 3 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
const assert = require('assert')
const net = require('net')
const util = require('./core/util')
const timers = require('./timers')
const Request = require('./core/request')
const DispatcherBase = require('./dispatcher-base')
const {
Expand Down Expand Up @@ -444,9 +445,9 @@ class Parser {
setTimeout (value, type) {
this.timeoutType = type
if (value !== this.timeoutValue) {
clearTimeout(this.timeout)
timers.clearTimeout(this.timeout)
if (value) {
this.timeout = setTimeout(onParserTimeout, value, this)
this.timeout = timers.setTimeout(onParserTimeout, value, this)
// istanbul ignore else: only for jest
if (this.timeout.unref) {
this.timeout.unref()
Expand Down Expand Up @@ -562,7 +563,7 @@ class Parser {
this.llhttp.llhttp_free(this.ptr)
this.ptr = null

clearTimeout(this.timeout)
timers.clearTimeout(this.timeout)
this.timeout = null
this.timeoutValue = null
this.timeoutType = null
Expand Down
89 changes: 89 additions & 0 deletions lib/timers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
'use strict'

let fastNow = Date.now()
let fastNowTimeout

const fastTimers = []

function onTimeout () {
fastNow = Date.now()

let len = fastTimers.length
let idx = 0
while (idx < len) {
const timer = fastTimers[idx]

if (timer.expires && fastNow >= timer.expires) {
timer.expires = 0
timer.callback(timer.opaque)
}

if (timer.expires === 0) {
timer.active = false
if (idx !== len - 1) {
fastTimers[idx] = fastTimers.pop()
} else {
fastTimers.pop()
}
len -= 1
} else {
idx += 1
}
}

if (fastTimers.length > 0) {
refreshTimeout()
}
}

function refreshTimeout () {
if (fastNowTimeout && fastNowTimeout.refresh) {
ronag marked this conversation as resolved.
Show resolved Hide resolved
fastNowTimeout.refresh()
} else {
clearTimeout(fastNowTimeout)
fastNowTimeout = setTimeout(onTimeout, 1e3)
if (fastNowTimeout.unref) {
fastNowTimeout.unref()
}
}
}

class Timeout {
constructor (callback, delay, opaque) {
this.callback = callback
this.delay = delay
this.opaque = opaque
this.expires = 0
this.active = false

this.refresh()
}

refresh () {
if (!this.active) {
this.active = true
fastTimers.push(this)
if (!fastNowTimeout || fastTimers.length === 1) {
refreshTimeout()
ronag marked this conversation as resolved.
Show resolved Hide resolved
fastNow = Date.now()
}
}

this.expires = fastNow + this.delay
}

clear () {
this.expires = 0
}
}

module.exports = {
setTimeout (callback, delay, opaque) {
return new Timeout(callback, delay, opaque)
},
clearTimeout (timeout) {
if (timeout && timeout.clear) {
timeout.clear()
}
}
}
7 changes: 7 additions & 0 deletions test/client-keep-alive.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const { test } = require('tap')
const { Client } = require('..')
const timers = require('../lib/timers')
const { kConnect } = require('../lib/core/symbols')
const { createServer } = require('net')
const http = require('http')
Expand Down Expand Up @@ -47,6 +48,12 @@ test('keep-alive header 0', (t) => {
const clock = FakeTimers.install()
t.teardown(clock.uninstall.bind(clock))

const orgTimers = { ...timers }
Object.assign(timers, { setTimeout, clearTimeout })
t.teardown(() => {
Object.assign(timers, orgTimers)
})

const server = createServer((socket) => {
socket.write('HTTP/1.1 200 OK\r\n')
socket.write('Content-Length: 0\r\n')
Expand Down
7 changes: 7 additions & 0 deletions test/client-reconnect.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const { test } = require('tap')
const { Client } = require('..')
const { createServer } = require('http')
const FakeTimers = require('@sinonjs/fake-timers')
const timers = require('../lib/timers')

test('multiple reconnect', (t) => {
t.plan(5)
Expand All @@ -12,6 +13,12 @@ test('multiple reconnect', (t) => {
const clock = FakeTimers.install()
t.teardown(clock.uninstall.bind(clock))

const orgTimers = { ...timers }
Object.assign(timers, { setTimeout, clearTimeout })
t.teardown(() => {
Object.assign(timers, orgTimers)
})

const server = createServer((req, res) => {
n === 0 ? res.destroy() : res.end('ok')
})
Expand Down
15 changes: 14 additions & 1 deletion test/client-timeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const { Client, errors } = require('..')
const { createServer } = require('http')
const { Readable } = require('stream')
const FakeTimers = require('@sinonjs/fake-timers')
const timers = require('../lib/timers')

test('refresh timeout on pause', (t) => {
t.plan(1)
Expand Down Expand Up @@ -51,6 +52,12 @@ test('start headers timeout after request body', (t) => {
const clock = FakeTimers.install()
t.teardown(clock.uninstall.bind(clock))

const orgTimers = { ...timers }
Object.assign(timers, { setTimeout, clearTimeout })
t.teardown(() => {
Object.assign(timers, orgTimers)
})

const server = createServer((req, res) => {
})
t.teardown(server.close.bind(server))
Expand Down Expand Up @@ -101,6 +108,12 @@ test('start headers timeout after async iterator request body', (t) => {
const clock = FakeTimers.install()
t.teardown(clock.uninstall.bind(clock))

const orgTimers = { ...timers }
Object.assign(timers, { setTimeout, clearTimeout })
t.teardown(() => {
Object.assign(timers, orgTimers)
})

const server = createServer((req, res) => {
})
t.teardown(server.close.bind(server))
Expand Down Expand Up @@ -167,7 +180,7 @@ test('parser resume with no body timeout', (t) => {
onConnect () {
},
onHeaders (statusCode, headers, resume) {
setTimeout(resume, 100)
setTimeout(resume, 2000)
return false
},
onData () {
Expand Down
7 changes: 7 additions & 0 deletions test/fetch/fetch-timeouts.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const { test } = require('tap')

const { fetch, Agent } = require('../..')
const timers = require('../../lib/timers')
const { createServer } = require('http')
const FakeTimers = require('@sinonjs/fake-timers')

Expand All @@ -16,6 +17,12 @@ test('Fetch very long request, timeout overridden so no error', (t) => {
const clock = FakeTimers.install()
t.teardown(clock.uninstall.bind(clock))

const orgTimers = { ...timers }
Object.assign(timers, { setTimeout, clearTimeout })
t.teardown(() => {
Object.assign(timers, orgTimers)
})

const server = createServer((req, res) => {
setTimeout(() => {
res.end('hello')
Expand Down