jsdelivr / api

DEPRECATED - API for public CDNs
http://api.jsdelivr.com
MIT License
112 stars 29 forks source link

Enable gzip #17

Closed bebraw closed 9 years ago

tomByrer commented 10 years ago

& perhaps minify JSON (not sure if the extra whitespace is from browsers' JavaScript-API)? edit: Basically just don't include \r \n \t \s (if they exist).

bebraw commented 10 years ago

& perhaps minify JSON (not sure if the extra whitespace is from browsers' JavaScript-API)? Basically just remove \r \n \t \s (if they exist).

I can accept a PR for this. Probably the cleanest way is to implement it as a Connect middleware so you don't need to touch rest.

jimaek commented 10 years ago

Our proxy already gzips it I think. Minifications should be an extra parameter and by default it should output the clean human version. What do you think?

EnigmaSolved commented 10 years ago

Edit: I goofed! I thought we were talking about the CDN (I had missed that this issue was in the API repo).

I just did a quick check of http://cdn.jsdelivr.net/jquery/2.1.1/jquery.min.js and Live HTTP Headers reports Content-Encoding: gzip, so yeah, it looks to me like gzip is already working. :)

And by "extra parameter" (for minification), @jimaek, I assume you mean the .min.js, which I agree is a good way to handle things (and seems pretty standard).

jimaek commented 10 years ago

@EnigmaSolved This is the API repo so we are talking about http://api.jsdelivr.com/v1/jsdelivr/libraries/jquery and not the CDN :)

EnigmaSolved commented 10 years ago

@jimaek: Oops--My bad! I came across jsDelivr a month or so ago, and in perusing the Git repos came across the current thread and so held off on doing anything else (since I interpreted gzip was not yet supported), and in the process missed that I was in the API repo (and that gzip is supported for the CDN). Thanks for bearing with my misinformation above! I'm really looking forward to switching to jsDelivr for my website! :)

tomByrer commented 10 years ago

extra parameter and by default it should output the clean human version

I'm not sure of any real reason for a human-readable version by default, can you give a use-case please? & the little usefulness for a toggle to turn on API-based beautification is mostly for us for in-browser testing. People should only look at raw API data when they're developing, & even then not always needed there. Worse comes to worse there are many online JSON beautifiers & tree-viewers.

I can accept a PR for this. Probably the cleanest way is to implement it as a Connect middleware

Thanks, but I don't know Connect currently, sorry. :/ A quick look at your code does not show where you add whitespace, perhaps in res.json? But that should default to 0 unless still in dev mode? Leaving the last parameter blank in JSON.stringify prevents whitespace being added.

bebraw commented 10 years ago

A quick look at your code does not show where you add whitespace, perhaps in res.json?

It's the default behavior of res.json (JSON.stringify). You can configure like this: app.set('json spaces', null); (no newlines). This is global configuration and doesn't fit this particular case that well.

It is cleanest to implement it as a middleware evaluated at the end of the stack. Simply strip newlines from res there and hit next().

jimaek commented 10 years ago

I'm not sure of any real reason for a human-readable version by default, can you give a use-case please?

To simplify the work for the developer. When testing the API he will try opening it himself and see what data is returned with different parameters. And once he is ready to go live he can add the parameter to return minified data.

bebraw commented 10 years ago

Please open a separate issue for the extra parameter and specify what it should look like there. I'll get the feature done.

tomByrer commented 10 years ago

To simplify the work for the developer

I get that, but this dev-traffic ideally should only represent only 0.001% of all traffic, right? (Right now is weird since API is still semi-beta.) So the default is for 0.001% & the flag/exception is for 99.999%?

bebraw commented 10 years ago

I get that, but this dev-traffic ideally should only represent only 0.001% of all traffic, right? (Right now is weird since API is still semi-beta.) So the default is for 0.001% & the flag/exception is for 99.999%?

I agree that making the output minified (no \n's) wouldn't break anything and would save a bit of bandwidth (worth it?).

It would be interesting to know how other APIs deal with the same issue.

tomByrer commented 10 years ago

[no whitespace will] save a bit of bandwidth (worth it?).

Some, enough to be worth it for medium to large queries, but not as much as you assume since gzip will tokenize whitespace really well; there are alot of ",\n, '{\n etc. Here is a test, not sure if my GZip parameters are the same on your server that I used in 7-zip (normal level, dictionary 32k, Word 32), but here is a test of jsDelivr name=jq* (full jsDelivr dump breaks my computer)

405,190 jsdelivr.jq-splat.src.json
 55,298 jsdelivr.jq-splat.src.json.gz
289,494 jsdelivr.jq-splat.min.json
 51,182 jsdelivr.jq-splat.min.json.gz

I think the bigger impact will be on server & client resources; smaller files are faster to pack, unpack, process, take less HD space. 1/4 unpacked smaller file size is significant.

It would be interesting to know how other APIs deal with the same issue

CDNJS is whitespaced; d/l it makes my laptop's fans go into turbo.

I discovered a good middle-ground between readable & compressed by removing whitespace around brackets, but leaving newlines around data:

[{
"name":"AngularJS",
"_id":"l1IY4GwpT",
"created":"2014-05-06T06:53:53.685Z",
"mainfile":"angular.min.js",
"homepage":"http://angularjs.org",
"lastversion":"1.3.0-beta.5",
"assets":
[{
"version":"1.3.0-beta.5",
"files":
[
"angular.js",
"angular.min.js"
]},{
"version":"1.3.0-beta.4",
"files":
[
"angular.js",
"angular.min.js"
]},{
"version":"1.3.0-beta.3",

IMHO the normal output should still remove all whitespace, but this semi-minified can be used to save resources for the human-readable dev output. I find this is still eye-scanable, since data is chunked well I can speed-read it. (Actually easier, since I can read down instead of back & forth with indented data.)

jimaek commented 9 years ago

We already do Gzip thanks to MaxCDN that is in front of the API server. I think we can close this.