♻️Refactor: Simplify content negotiation code #2865

nickajacks1 · 2024-02-19T00:13:06Z

Description

The implementation of forEachParameter was upstreamed to fasthttp, so use that version instead of maintaining our own.

The previous implementation of content negotiation used some difficult to understand techniques in order to reduce allocations, potentially hurting maintainability. The more natural approach for storing and comparing unordered media-type parameters is to utilize maps. While the resulting code still isn't as simple as it could be, it's a step closer.

To reduce allocations, we use a sync.Pool. This actually reduces in fewer allocations than before at 3 or more parameters. The net result is nearly identical performance for zero parameters, almost-as-good performance for 1-2 parameters, and better performance for 3+ parameters.

Also tested performance using k6, see below for details.

Changes Introduced

List the new features or adjustments introduced in this pull request. Provide details on benchmarks, documentation updates, changelog entries, and if applicable, the migration guide.

There is no API change in this PR.

Benchmarks: Describe any performance benchmarks and improvements related to the changes.

Type of Change

Please delete options that are not relevant.

Enhancement (improvement to existing features and functionality)
Code consistency (non-breaking change which improves code reliability and robustness)

Checklist

Before you submit your pull request, please make sure you meet these requirements:

Conducted a self-review of the code and provided comments for complex or critical parts.
Added or updated unit tests to validate the effectiveness of the changes or new features.
Ensured that new and existing unit tests pass locally with the changes.
Aimed for optimal performance with minimal allocations in the new code.
Provided benchmarks for the new code to analyze and improve upon.

Benchmarks

                                                                   │ ../fiber2/main.txt │              branch.txt              │
                                                                   │       sec/op       │    sec/op     vs base                │
_Ctx_Accepts/run-[]string{".xml"}-8                                         168.7n ± 1%   170.8n ±  2%        ~ (p=0.066 n=10)
_Ctx_Accepts/run-[]string{"json",_"xml"}-8                                  219.7n ± 3%   225.0n ±  1%   +2.44% (p=0.015 n=10)
_Ctx_Accepts/run-[]string{"application/json",_"application/xml"}-8          168.4n ± 3%   181.2n ±  1%   +7.54% (p=0.000 n=10)
_Ctx_AcceptsCharsets-8                                                      78.06n ± 1%   81.70n ±  2%   +4.68% (p=0.000 n=10)
_Ctx_AcceptsEncodings-8                                                     107.4n ± 2%   123.2n ±  2%  +14.61% (p=0.000 n=10)
_Ctx_AcceptsLanguages-8                                                     167.6n ± 9%   184.8n ±  3%  +10.26% (p=0.008 n=10)
_Utils_GetOffer/simple-8                                                    34.15n ± 1%   36.23n ±  3%   +6.09% (p=0.000 n=10)
_Utils_GetOffer/6_offers-8                                                  82.80n ± 3%   89.80n ±  3%   +8.46% (p=0.000 n=10)
_Utils_GetOffer/1_parameter-8                                               103.7n ± 2%   159.5n ±  2%  +53.81% (p=0.000 n=10)
_Utils_GetOffer/2_parameters-8                                              176.5n ± 1%   222.5n ±  2%  +26.06% (p=0.000 n=10)
_Utils_GetOffer/3_parameters-8                                              349.1n ± 0%   305.1n ±  2%  -12.63% (p=0.000 n=10)
_Utils_GetOffer/10_parameters-8                                             1.325µ ± 1%   1.064µ ±  1%  -19.71% (p=0.000 n=10)
_Utils_GetOffer/6_offers_w/params-8                                         267.6n ± 2%   314.4n ±  2%  +17.51% (p=0.000 n=10)
_Utils_GetOffer/mime_extension-8                                            201.2n ± 1%   212.7n ±  1%   +5.74% (p=0.000 n=10)
_Utils_GetOffer/mime_extension#01-8                                         274.9n ± 1%   278.6n ±  1%   +1.35% (p=0.000 n=10)
_Utils_GetOffer/mime_extension#02-8                                         420.7n ± 1%   417.9n ±  0%   -0.65% (p=0.030 n=10)
_Utils_GetOffer/mime_extension#03-8                                         184.6n ± 2%   191.9n ±  3%   +3.93% (p=0.000 n=10)
_Utils_GetOffer/web_browser-8                                               114.2n ± 2%   130.4n ± 11%  +14.19% (p=0.000 n=10)
geomean                                                                     176.5n        189.0n         +7.08%

                                                                   │ ../fiber2/main.txt │               branch.txt                │
                                                                   │        B/op        │    B/op     vs base                     │
_Ctx_Accepts/run-[]string{".xml"}-8                                        0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Ctx_Accepts/run-[]string{"json",_"xml"}-8                                 0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Ctx_Accepts/run-[]string{"application/json",_"application/xml"}-8         0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Ctx_AcceptsCharsets-8                                                     0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Ctx_AcceptsEncodings-8                                                    0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Ctx_AcceptsLanguages-8                                                    0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/simple-8                                                   0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/6_offers-8                                                 0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/1_parameter-8                                              0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/2_parameters-8                                             0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/3_parameters-8                                             128.0 ± 0%       0.0 ± 0%  -100.00% (p=0.000 n=10)
_Utils_GetOffer/10_parameters-8                                          896.000 ± 0%     7.000 ± 0%   -99.22% (p=0.000 n=10)
_Utils_GetOffer/6_offers_w/params-8                                        0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/mime_extension-8                                           0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/mime_extension#01-8                                        48.00 ± 0%     48.00 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/mime_extension#02-8                                        48.00 ± 0%     48.00 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/mime_extension#03-8                                        0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/web_browser-8                                              0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
geomean                                                                               ²               ?                       ² ³
¹ all samples are equal
² summaries must be >0 to compute geomean
³ ratios must be >0 to compute geomean

                                                                   │ ../fiber2/main.txt │               branch.txt                │
                                                                   │     allocs/op      │ allocs/op   vs base                     │
_Ctx_Accepts/run-[]string{".xml"}-8                                        0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Ctx_Accepts/run-[]string{"json",_"xml"}-8                                 0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Ctx_Accepts/run-[]string{"application/json",_"application/xml"}-8         0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Ctx_AcceptsCharsets-8                                                     0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Ctx_AcceptsEncodings-8                                                    0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Ctx_AcceptsLanguages-8                                                    0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/simple-8                                                   0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/6_offers-8                                                 0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/1_parameter-8                                              0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/2_parameters-8                                             0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/3_parameters-8                                             1.000 ± 0%     0.000 ± 0%  -100.00% (p=0.000 n=10)
_Utils_GetOffer/10_parameters-8                                            3.000 ± 0%     0.000 ± 0%  -100.00% (p=0.000 n=10)
_Utils_GetOffer/6_offers_w/params-8                                        0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/mime_extension-8                                           0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/mime_extension#01-8                                        2.000 ± 0%     2.000 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/mime_extension#02-8                                        2.000 ± 0%     2.000 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/mime_extension#03-8                                        0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
_Utils_GetOffer/web_browser-8                                              0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹

k6 benchmarks

Server

package main

import (
	"github.com/gofiber/fiber/v3"
)

type Gorby struct {
	Name string `json:"name"`
	Age  int    `json:"age"`
}

func main() {
	app := fiber.New()

	gorby := Gorby{
		"Genrie",
		39,
	}

	app.Get("/hello", func(c fiber.Ctx) error {
		return c.Format(
			fiber.ResFmt{
				MediaType: "application/json",
				Handler: func(fiber.Ctx) error {
					return c.SendStatus(406)
				},
			},
			fiber.ResFmt{
				MediaType: "application/json;version=1",
				Handler: func(c fiber.Ctx) error {
					return c.JSON(gorby)
				},
			},
			fiber.ResFmt{
				MediaType: "text/html",
				Handler: func(c fiber.Ctx) error {
					return c.SendString("<p>Hey genroid</p>")
				},
			},
		)
	})

	app.Listen(":3002")
}

Script

import http from 'k6/http';
import { randomIntBetween } from 'https://jslib.k6.io/k6-utils/1.4.0/index.js';

export const options = {
  vus: 25,
  duration: '30s',
};

const accepts = [
  'application/json',
  'text/plain;q=0.89,application/json;version=1;q=0.91,text/html;q=0.90',
  'application/json;version=1',
  '*/*',
  'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8',
];

export default function() {
  const idx = randomIntBetween(0, accepts.length-1)
  const params = {
    headers: {
      'Accept': accepts[idx],
    },
  };

  http.get('http://localhost:3002/hello', params);
}

PR

     data_received..................: 374 MB  13 MB/s
     data_sent......................: 336 MB  11 MB/s
     http_req_blocked...............: avg=1.41µs   min=369ns   med=916ns    max=14.77ms p(90)=1.31µs   p(95)=1.64µs
     http_req_connecting............: avg=6ns      min=0s      med=0s       max=1.64ms  p(90)=0s       p(95)=0s
     http_req_duration..............: avg=240.6µs  min=16.01µs med=186.84µs max=33.44ms p(90)=464.53µs p(95)=592.14µs
       { expected_response:true }...: avg=238.93µs min=19.27µs med=185.3µs  max=32.55ms p(90)=462.36µs p(95)=589.51µs
     http_req_failed................: 40.00%  ✓ 1031450      ✗ 1546962
     http_req_receiving.............: avg=16.67µs  min=3.99µs  med=10.05µs  max=33.36ms p(90)=19.44µs  p(95)=38.11µs
     http_req_sending...............: avg=6.76µs   min=1.8µs   med=4.29µs   max=24.95ms p(90)=6.48µs   p(95)=8.9µs
     http_req_tls_handshaking.......: avg=0s       min=0s      med=0s       max=0s      p(90)=0s       p(95)=0s
     http_req_waiting...............: avg=217.17µs min=8.95µs  med=169.48µs max=23.36ms p(90)=440.1µs  p(95)=563.63µs
     http_reqs......................: 2578412 85946.428171/s
     iteration_duration.............: avg=283.64µs min=29.92µs med=222.84µs max=33.48ms p(90)=511.42µs p(95)=644.03µs
     iterations.....................: 2578412 85946.428171/s
     vus............................: 25      min=25         max=25
     vus_max........................: 25      min=25         max=25


running (0m30.0s), 00/25 VUs, 2578412 complete and 0 interrupted iterations

Main Branch

     data_received..................: 375 MB  13 MB/s
     data_sent......................: 336 MB  11 MB/s
     http_req_blocked...............: avg=1.42µs   min=349ns   med=913ns    max=20.02ms p(90)=1.3µs    p(95)=1.65µs
     http_req_connecting............: avg=2ns      min=0s      med=0s       max=3.18ms  p(90)=0s       p(95)=0s
     http_req_duration..............: avg=240.04µs min=17.52µs med=185.9µs  max=30.31ms p(90)=464.42µs p(95)=593.65µs
       { expected_response:true }...: avg=238.39µs min=17.52µs med=184.41µs max=27.29ms p(90)=463.27µs p(95)=592µs
     http_req_failed................: 40.00%  ✓ 1033268      ✗ 1549422
     http_req_receiving.............: avg=16.6µs   min=3.91µs  med=10.05µs  max=30.26ms p(90)=19.85µs  p(95)=38.15µs
     http_req_sending...............: avg=6.73µs   min=1.87µs  med=4.26µs   max=22.89ms p(90)=6.59µs   p(95)=9µs
     http_req_tls_handshaking.......: avg=0s       min=0s      med=0s       max=0s      p(90)=0s       p(95)=0s
     http_req_waiting...............: avg=216.7µs  min=9.89µs  med=168.56µs max=20.27ms p(90)=439.96µs p(95)=565.18µs
     http_reqs......................: 2582690 86089.110158/s
     iteration_duration.............: avg=283.02µs min=32.25µs med=222.01µs max=35.06ms p(90)=511.66µs p(95)=646.09µs
     iterations.....................: 2582690 86089.110158/s
     vus............................: 25      min=25         max=25
     vus_max........................: 25      min=25         max=25


running (0m30.0s), 00/25 VUs, 2582690 complete and 0 interrupted iterations

The implementation of forEachParameter was upstreamed to fasthttp, so use that version instead of maintaining our own.

The previous implementation of content negotiation used some difficult to understand techniques in order to reduce allocations, potentially hurting maintainability. The more natural approach for storing and comparing unordered media-type parameters is to utilize maps. While the resulting code still isn't as simple as it could be, it's a step closer. To reduce allocations, we use a sync.Pool. This actually reduces in fewer allocations than before at 3 or more parameters. The net result is nearly identical performance for zero parameters, almost-as-good performance for 1-2 parameters, and better performance for 3+ parameters.

helpers.go

sixcolors · 2024-02-19T03:42:38Z

I haven't had the chance to do a full review as I’m 🤧 and examining the PR on mobile. I'll conduct a more in-depth analysis before approving the PR.

Overall the changes appear positive. The one thing I’m seeing is, considering that real-world scenarios often involve fewer than three parameters, the benchmarks suggest a more noticeable performance reduction. The performance trade-off seems to range from approximately 8% to 54% for these scenarios.

nickajacks1 · 2024-02-19T04:33:36Z

Thanks for reviewing.
Indeed, there's an increased latency on my machine by a few dozen nanoseconds. To put that into perspective, it doesn't register as a blip on my laptop running a server that is doing nothing but content negotiation. On more powerful machines running services that do actual work, I think we'd have a more difficult time finding a difference. If nanoseconds mattered, you'd probably want to avoid content negotiation in the first place. Individual benchmarks can be an excellent metric, but they are by definition very narrow in scope, and I find they can distract from more juicy areas if you're not careful.

That said, the biggest reason I did this refactor was for code clarity, so it's probably only worth the slice => map refactor if it leads to easier to understand code. The switch to fasthttp.VisitHeaderParams can be done separately.

I can also repeat the same k6 benchmarks on some weaker armv7 embedded linux devices I have access to in order to see more exaggerated circumstances if we'd like to see more varied statistics.

nickajacks1 added 2 commits February 16, 2024 17:42

refactor: Update to use fasthttp.VisitHeaderParams

8c0e565

The implementation of forEachParameter was upstreamed to fasthttp, so use that version instead of maintaining our own.

nickajacks1 requested a review from a team as a code owner February 19, 2024 00:13

nickajacks1 requested review from gaby, sixcolors, ReneWerner87 and efectn and removed request for a team February 19, 2024 00:13

Merge branch 'main' into fasthttp-headerparams

f9f28ae

sixcolors reviewed Feb 19, 2024

View reviewed changes

helpers.go Show resolved Hide resolved

ReneWerner87 approved these changes Feb 19, 2024

View reviewed changes

ReneWerner87 added 🧹 Updates v3 labels Feb 19, 2024

efectn approved these changes Feb 19, 2024

View reviewed changes

ReneWerner87 merged commit 8ca562c into gofiber:main Feb 19, 2024
17 checks passed

nickajacks1 deleted the fasthttp-headerparams branch February 19, 2024 19:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

♻️Refactor: Simplify content negotiation code #2865

♻️Refactor: Simplify content negotiation code #2865

nickajacks1 commented Feb 19, 2024

sixcolors commented Feb 19, 2024 •

edited

Loading

nickajacks1 commented Feb 19, 2024

♻️Refactor: Simplify content negotiation code #2865

♻️Refactor: Simplify content negotiation code #2865

Conversation

nickajacks1 commented Feb 19, 2024

Description

Changes Introduced

Type of Change

Checklist

Server

Script

PR

Main Branch

sixcolors commented Feb 19, 2024 • edited Loading

nickajacks1 commented Feb 19, 2024

sixcolors commented Feb 19, 2024 •

edited

Loading