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

Add NotContentTypes option #81

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jared2501
Copy link
Contributor

Use case: I would like to gzip all content-types, except images/videos.

@jared2501
Copy link
Contributor Author

hey @fsouza, @jprobinson - would ya'll be interested in this change? If so, happy to add tests / refactor.

@jprobinson
Copy link
Contributor

@jared2501 this seems like a reasonable addition to me. The code looks like it's in a good place. Go ahead and add some tests and I'll bring this in. Thanks!

@fsouza
Copy link
Contributor

fsouza commented Feb 25, 2019

oh sorry I missed this. Yeah I second @jp's comment and I was actually thinking about this over the weekend (we have a gizmo service that generates video thumbnails and we don't want to gzip jpg images :D).

@adammck
Copy link
Contributor

adammck commented Feb 25, 2019

Hi! Because I like functional options so much, I might personally implement this api as:

interface contentTypeMatcher {
  Matches(contentType string) bool
}

func ContentTypeMatchers(matchers ... contentTypeMatcher) option {
   // ...
}

gzip.GzipHandlerWithOpts(gzip.ContentTypeMatchers(
  gzip.MatchLiteral("text/plain"),
  gzip.MatchLiteral("image/jpg", invert=True), // does go have named params? i forgot
))

...so that in some magical future, you might do such weird things as:

gzip.MatchPrefix("text/")
gzip.MatchRegex("*/*+json")
gzip.MatchFunc(func(ct string) bool {
  return time.Now().Hour < 12
})

but then I'd probably add a helper that looks just like what you're proposing, so I dunno.

@mkraft
Copy link

mkraft commented May 9, 2020

I have opened a PR for this also: #98

klauspost added a commit to klauspost/compress that referenced this pull request May 31, 2021
klauspost added a commit to klauspost/compress that referenced this pull request Jun 2, 2021
Fork and clean up+extend the dead `nytimes/gziphandler` project.

* Adds `http.Transport` wrapper.
* Includes nytimes/gziphandler#106 as well as support for stateless encoding.
* Implements a variant of nytimes/gziphandler#81
* Fixes nytimes/gziphandler#103
* Strip "Accept-Ranges" on compressed content. Fixes nytimes/gziphandler#83
* Removes testify from deps.
* Constructors boiled down.
* Defaults to this gzip package.
* Allocations reduced.

Default settings comparison:
```
λ benchcmp before.txt after.txt                                        
benchmark                         old ns/op     new ns/op     delta    
BenchmarkGzipHandler_S2k-32       51302         25554         -50.19%  
BenchmarkGzipHandler_S20k-32      301426        174900        -41.98%  
BenchmarkGzipHandler_S100k-32     1546203       912349        -40.99%  
BenchmarkGzipHandler_P2k-32       3973          2116          -46.74%  
BenchmarkGzipHandler_P20k-32      20319         12237         -39.78%  
BenchmarkGzipHandler_P100k-32     96079         57348         -40.31%  
                                                                       
benchmark                         old MB/s     new MB/s     speedup    
BenchmarkGzipHandler_S2k-32       39.92        80.14        2.01x      
BenchmarkGzipHandler_S20k-32      67.94        117.10       1.72x      
BenchmarkGzipHandler_S100k-32     66.23        112.24       1.69x      
BenchmarkGzipHandler_P2k-32       515.44       967.76       1.88x      
BenchmarkGzipHandler_P20k-32      1007.92      1673.55      1.66x      
BenchmarkGzipHandler_P100k-32     1065.79      1785.58      1.68x      
                                                                       
benchmark                         old allocs     new allocs     delta  
BenchmarkGzipHandler_S2k-32       22             19             -13.64%
BenchmarkGzipHandler_S20k-32      25             22             -12.00%
BenchmarkGzipHandler_S100k-32     28             25             -10.71%
BenchmarkGzipHandler_P2k-32       22             19             -13.64%
BenchmarkGzipHandler_P20k-32      25             22             -12.00%
BenchmarkGzipHandler_P100k-32     27             24             -11.11%
```

Client Transport:

Speed compared to standard library for an approximate 127KB payload:

```
BenchmarkTransport

Single core:
BenchmarkTransport/gzhttp-32         	    1995	    609791 ns/op	 214.14 MB/s	   10129 B/op	      73 allocs/op
BenchmarkTransport/stdlib-32         	    1567	    772161 ns/op	 169.11 MB/s	   53950 B/op	      99 allocs/op

Multi Core:
BenchmarkTransport/gzhttp-par-32     	   29113	     36802 ns/op	3548.27 MB/s	   11061 B/op	      73 allocs/op
BenchmarkTransport/stdlib-par-32     	   16114	     66442 ns/op	1965.38 MB/s	   54971 B/op	      99 allocs/op
```

This includes both serving the http request, parsing requests and decompressing.
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.

5 participants