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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

splice isn't used when serving files #60

Closed
ghz-max opened this issue Apr 2, 2022 · 3 comments 路 Fixed by #63
Closed

splice isn't used when serving files #60

ghz-max opened this issue Apr 2, 2022 · 3 comments 路 Fixed by #63
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@ghz-max
Copy link

ghz-max commented Apr 2, 2022

馃挕 The feature or bug you are proposing

splice isn't used when serving static files.

馃搫 The description of the bug or the logic behind your proposal

I have a special use case where I need to serve files from a local http server. Golang has a special behavior when the destination writer is a https://pkg.go.dev/net#TCPConn.ReadFrom and your source is a "splice-able" reader, like a file or a socket on a Linux host.

Steps to reproduce:

  1. Download a large enough file (few hundreds MiB) and store it in /home/dev/Downloads
  2. Run a bunrouter app serving static files on port 8083
package main

import (
	"flag"
	"log"
	"net/http"

	"github.com/uptrace/bunrouter"
	"github.com/uptrace/bunrouter/extra/reqlog"
)

var (
	dir = flag.String("dir", "/home/dev/Downloads", "file directory")
)

func main() {
	flag.Parse()

	router := bunrouter.New(
		bunrouter.Use(reqlog.NewMiddleware(reqlog.WithVerbose(false))),
	)

	fileServer := http.FileServer(http.Dir(*dir))
	fileServer = http.StripPrefix("/static", fileServer)

	router.GET("/static/*path", bunrouter.HTTPHandler(fileServer))

	log.Println("listening on http://localhost:8083 - serving ", *dir)
	log.Println(http.ListenAndServe(":8083", router))
}
  1. Run a net/http app running on port 8082
package main

import (
	"flag"
	"log"
	"net/http"
)

var (
	dir = flag.String("dir", "/home/dev/Downloads", "file directory")
)

func main() {
	flag.Parse()

	fs := http.FileServer(http.Dir(*dir))
	http.Handle("/static/", http.StripPrefix("/static", fs))

	log.Println("listening on http://localhost:8082 - serving ", *dir)
	log.Println(http.ListenAndServe(":8082", nil))
}
  1. Use a http benchmark tool like wrk, ab or h2load
    e.g. :
  1. Results on a 16 cores host with each core at 100%:
  • bunrouter: 16GiB/s
  • net/http: 44GiB/s

The whole point of using splice is to avoid unnecessary cpu cycle.

馃殌 The expected result

bunrouter uses https://pkg.go.dev/net#TCPConn.ReadFrom which allow splice to be used.

@vmihailenco
Copy link
Member

Try to remove reqlog.NewMiddleware - it wraps http.ResponseWriter to enable logging.

@ghz-max
Copy link
Author

ghz-max commented Apr 3, 2022

Thanks for the quick answer!

You are right, without reqlog middleware, the speed is as expected.

@vmihailenco vmihailenco added the enhancement New feature or request label Apr 4, 2022
@vmihailenco
Copy link
Member

I guess we could add a similar wrapper to expose ReadFrom in reqlog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants