Skip to content

Conversation

usualoma
Copy link
Member

@usualoma usualoma commented May 17, 2025

This PR adopts usualoma#3, excludes #241, and completely removes the functionality of getInternalBody.

support more response body types for caching

In d72de8d, I made the optimization pattern previously covered by getInternalBody return from a faster cache.

benchmark

script

import { serve } from '@hono/node-server'
import { Hono } from 'hono'

const app = new Hono()

app.get('/', () => {
    const res = new Response('foo')
    res.headers.set('x-foo', 'bar')
    return res
})

app.get('/uint8array', () => {
    return new Response(new Uint8Array([102, 111, 111])) // prints "foo"
})

app.get('/blob', () => {
    return new Response(new Blob([new Uint8Array([102, 111, 111])])) // prints "foo"
})

serve({
    fetch: app.fetch,
    port: 3003,
})

main branch

$ bombardier -d 10s --fasthttp http://localhost:3003/
Bombarding http://localhost:3003/ for 10s using 125 connection(s)
[==========================================================================] 10s
Done!
Statistics        Avg      Stdev        Max
  Reqs/sec     41208.28    6649.70   51394.11
  Latency        3.03ms     3.18ms   328.28ms
  HTTP codes:
    1xx - 0, 2xx - 411938, 3xx - 0, 4xx - 0, 5xx - 0
    others - 0
  Throughput:     9.39MB/s
$ bombardier -d 10s --fasthttp http://localhost:3003/uint8array
Bombarding http://localhost:3003/uint8array for 10s using 125 connection(s)
[==========================================================================] 10s
Done!
Statistics        Avg      Stdev        Max
  Reqs/sec     39105.54    7149.83   48899.74
  Latency        3.19ms     1.58ms   143.78ms
  HTTP codes:
    1xx - 0, 2xx - 390974, 3xx - 0, 4xx - 0, 5xx - 0
    others - 0
  Throughput:     8.87MB/s
$ bombardier -d 10s --fasthttp http://localhost:3003/blob
Bombarding http://localhost:3003/blob for 10s using 125 connection(s)
[==========================================================================] 10s
Done!
Statistics        Avg      Stdev        Max
  Reqs/sec     34814.79    7252.95   48210.01
  Latency        3.59ms     1.25ms    83.12ms
  HTTP codes:
    1xx - 0, 2xx - 348063, 3xx - 0, 4xx - 0, 5xx - 0
    others - 0
  Throughput:     7.70MB/s

usualoma:feat-more-use-respose-cache-2 branch

$ bombardier -d 10s --fasthttp http://localhost:3003/
Bombarding http://localhost:3003/ for 10s using 125 connection(s)
[==========================================================================] 10s
Done!
Statistics        Avg      Stdev        Max
  Reqs/sec     50349.82    4485.60   61434.61
  Latency        2.48ms   412.72us    30.42ms
  HTTP codes:
    1xx - 0, 2xx - 503307, 3xx - 0, 4xx - 0, 5xx - 0
    others - 0
  Throughput:    11.52MB/s
$ bombardier -d 10s --fasthttp http://localhost:3003/uint8array
Bombarding http://localhost:3003/uint8array for 10s using 125 connection(s)
[==========================================================================] 10s
Done!
Statistics        Avg      Stdev        Max
  Reqs/sec     49739.84    6358.60   59130.66
  Latency        2.51ms     1.68ms   143.02ms
  HTTP codes:
    1xx - 0, 2xx - 497340, 3xx - 0, 4xx - 0, 5xx - 0
    others - 0
  Throughput:    11.29MB/s
$ bombardier -d 10s --fasthttp http://localhost:3003/blob
Bombarding http://localhost:3003/blob for 10s using 125 connection(s)
[==========================================================================] 10s
Done!
Statistics        Avg      Stdev        Max
  Reqs/sec     40411.95    6107.91   51111.33
  Latency        3.09ms     2.07ms   178.87ms
  HTTP codes:
    1xx - 0, 2xx - 403772, 3xx - 0, 4xx - 0, 5xx - 0
    others - 0
  Throughput:     8.94MB/s

@usualoma usualoma force-pushed the feat-more-use-respose-cache-2 branch from 74161ae to f657cae Compare May 17, 2025 01:58
Copy link
Member

@yusukebe yusukebe left a comment

Choose a reason for hiding this comment

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

LGTM!

@yusukebe
Copy link
Member

@usualoma I think this is good! Can I merge now?

@usualoma
Copy link
Member Author

@yusukebe Thank you. Please!

Can I merge now?

@yusukebe
Copy link
Member

Okay! Let's go.

@yusukebe yusukebe merged commit a8edc96 into honojs:main May 19, 2025
3 checks passed
@hamster1963
Copy link

Sorry to bother you all, but I noticed that after this pr merge, the streaming return no longer seems to work, in a similar situation to #244

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.

3 participants