Skip to content
This repository

10x speedup for cowboy_rest and dependents (eg cowboy_static) #305

Closed
wants to merge 2 commits into from

4 participants

Vladimir Dronnikov Loïc Hoguin Sergey Rublev Dmitry Groshev
Vladimir Dronnikov
dvv commented

httpd_util:rfc1123/1 is way slow. by introducing a faster ad hoc alternative a huge speedup is achieved.
please, consider applying.
tia, --vladimir

Loïc Hoguin
Owner

Yes, httpd_util dependency is to be removed, there's a ticket for that.

However, why do you replace rfc1123 by rfc2109?

Also io_lib:format is slow too.

Sergey Rublev

httpd_util:rfc1123_date converts local time to universal, it's for compliance with HTTP/1.1 RFC2616 wich says:
"All HTTP date/time stamps MUST be represented in Greenwich Mean Time
(GMT), without exception. For the purposes of HTTP, GMT is exactly
equal to UTC (Coordinated Universal Time)."

3> cowboy_clock:rfc2109_fast({{2012,11,01},{17,0,0}}).
<<"Thu, 01 Nov 2012 17:00:00 GMT">>
4> httpd_util:rfc1123_date({{2012,11,01},{17,0,0}}).
"Thu, 01 Nov 2012 10:00:00 GMT"`

hence, after the patch time returning in rest handler's, last_modified and expires, is not turning from local to UTC.

So, additional changes required in cowboy_static.erl for this patch:
from
Fileinfo = file:read_file_info(Filepath1),
to
Fileinfo = file:read_file_info(Filepath1, [{time, universal}]),

I don't presume to judge which one is preferable and more reasonable.

Vladimir Dronnikov

right, in my barebone static handler i use {time, universal}, so should imo do cowboy_static.
update: here is the crap\c\c\c\ccode

Vladimir Dronnikov

@essen i just added _fast suffix to what cowboy_clock exposed. haven't studied diff between both RFCs.

Loïc Hoguin
Owner

Ultimately we only want to deal with universal time, so this is definitely in the right direction, however the current rfc2109 function should be converted to this, it's only doing local time at the moment because cookies needed it, but cookies are a temporary implementation so the local code can be moved in the cookie module without any issue for now.

Dmitry Groshev

Can I suggest our lib for time formatting/parsing? It's quite battle tested and fast as hell: https://github.com/selectel/tempo/

Loïc Hoguin
Owner

No NIFs, sorry.

Loïc Hoguin
Owner

By the way this depends on #282. First fix the localtime that should always be UTC, then we can fix the parsing here properly.

Dmitry Groshev

Why don't use such small&pure NIFs like that one?

Loïc Hoguin
Owner

Because they mess up with the reduction counts, may crash, are harder to upgrade properly, etc. It's also not going to be faster than code written specifically for a datetime format, which only uses a single binary construct.

Loïc Hoguin
Owner

Done in 8bc6bde. Using the existing rfc1123 code from Cowboy. Dependency on httpd_util is gone!

Thanks for the initial patch and report!

Loïc Hoguin essen closed this
Loïc Hoguin
Owner

Oh and this didn't need to depend on #282. I was just misled. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
17 src/cowboy_clock.erl
@@ -25,7 +25,7 @@
25 25 -export([start_link/0]).
26 26 -export([stop/0]).
27 27 -export([rfc1123/0]).
28   --export([rfc2109/1]).
  28 +-export([rfc2109/1, rfc2109_fast/1]).
29 29
30 30 %% gen_server.
31 31 -export([init/1]).
@@ -106,6 +106,21 @@ rfc2109(LocalTime) ->
106 106 MinBin/binary, ":",
107 107 SecBin/binary, " GMT">>.
108 108
  109 +%% @doc Return the current date and time formatted according to RFC-2109.
  110 +%%
  111 +%% This format is used in the <em>set-cookie</em> header sent with
  112 +%% HTTP responses.
  113 +-spec rfc2109_fast(calendar:datetime()) -> binary().
  114 +rfc2109_fast(DateTime) ->
  115 + {{Year, Month, Day}, {Hour, Minute, Second}} = DateTime,
  116 + DoW = calendar:day_of_the_week(Year, Month, Day),
  117 + list_to_binary(io_lib:format(
  118 + "~s, ~2..0B ~s ~4..0B ~2..0B:~2..0B:~2..0B GMT", [
  119 + weekday(DoW),
  120 + Day, month(Month), Year,
  121 + Hour, Minute, Second
  122 + ])).
  123 +
109 124 %% gen_server.
110 125
111 126 %% @private
20 src/cowboy_req.erl
@@ -91,6 +91,7 @@
91 91 -export([set_resp_header/3]).
92 92 -export([set_resp_body/2]).
93 93 -export([set_resp_body_fun/3]).
  94 +-export([set_resp_body_fun/5]).
94 95 -export([has_resp_header/2]).
95 96 -export([has_resp_body/1]).
96 97 -export([delete_resp_header/2]).
@@ -836,6 +837,12 @@ set_resp_body(Body, Req) ->
836 837 set_resp_body_fun(StreamLen, StreamFun, Req) ->
837 838 Req#http_req{resp_body={StreamLen, StreamFun}}.
838 839
  840 +%% @see cowboy_req:set_resp_body_fun/3.
  841 +-spec set_resp_body_fun(non_neg_integer(), non_neg_integer(), non_neg_integer(), resp_body_fun(), Req)
  842 + -> Req when Req::req().
  843 +set_resp_body_fun(StreamStart, StreamEnd, StreamLen, StreamFun, Req) ->
  844 + Req#http_req{resp_body={StreamStart, StreamEnd, StreamLen, StreamFun}}.
  845 +
839 846 %% @doc Return whether the given header has been set for the response.
840 847 -spec has_resp_header(binary(), req()) -> boolean().
841 848 has_resp_header(Name, #http_req{resp_headers=RespHeaders}) ->
@@ -888,6 +895,19 @@ reply(Status, Headers, Body, Req=#http_req{
888 895 if RespType =/= hook, Method =/= <<"HEAD">> -> BodyFun();
889 896 true -> ok
890 897 end;
  898 + {Start, End, Length, BodyFun} ->
  899 + % partial response has 206 status
  900 + Status1 = case Status of 200 -> 206; _ -> Status end,
  901 + {RespType, Req2} = response(Status1, Headers, RespHeaders, [
  902 + {<<"content-length">>, integer_to_list(End - Start + 1)},
  903 + {<<"content-range">>,
  904 + io_lib:format("~B-~B/~B", [Start, End, Length])},
  905 + {<<"date">>, cowboy_clock:rfc1123()},
  906 + {<<"server">>, <<"Cowboy">>}
  907 + |HTTP11Headers], <<>>, Req),
  908 + if RespType =/= hook, Method =/= <<"HEAD">> -> BodyFun(Start, End - Start + 1);
  909 + true -> ok
  910 + end;
891 911 _ ->
892 912 {_, Req2} = response(Status, Headers, RespHeaders, [
893 913 {<<"content-length">>, integer_to_list(iolist_size(Body))},
6 src/cowboy_rest.erl
@@ -765,7 +765,7 @@ set_resp_body(Req, State=#state{content_type_a={_Type, Fun}}) ->
765 765 LastModified when is_atom(LastModified) ->
766 766 Req4 = Req3;
767 767 LastModified ->
768   - LastModifiedStr = httpd_util:rfc1123_date(LastModified),
  768 + LastModifiedStr = cowboy_clock:rfc2109_fast(LastModified),
769 769 Req4 = cowboy_req:set_resp_header(
770 770 <<"Last-Modified">>, LastModifiedStr, Req3)
771 771 end,
@@ -778,6 +778,8 @@ set_resp_body(Req, State=#state{content_type_a={_Type, Fun}}) ->
778 778 Req7 = case Body of
779 779 {stream, Len, Fun1} ->
780 780 cowboy_req:set_resp_body_fun(Len, Fun1, Req6);
  781 + {stream, Start, End, Length, Fun1} ->
  782 + cowboy_req:set_resp_body_fun(Start, End, Length, Fun1, Req6);
781 783 _Contents ->
782 784 cowboy_req:set_resp_body(Body, Req6)
783 785 end,
@@ -810,7 +812,7 @@ set_resp_expires(Req, State) ->
810 812 Expires when is_atom(Expires) ->
811 813 {Req2, State2};
812 814 Expires ->
813   - ExpiresStr = httpd_util:rfc1123_date(Expires),
  815 + ExpiresStr = cowboy_clock:rfc2109_fast(Expires),
814 816 Req3 = cowboy_req:set_resp_header(
815 817 <<"Expires">>, ExpiresStr, Req2),
816 818 {Req3, State2}
18 src/cowboy_static.erl
@@ -238,7 +238,7 @@ rest_init(Req, Opts) ->
238 238 etag_fun=ETagFunction};
239 239 ok ->
240 240 Filepath1 = join_paths(Directory1, Filepath),
241   - Fileinfo = file:read_file_info(Filepath1),
  241 + Fileinfo = file:read_file_info(Filepath1, [{time, universal}]),
242 242 #state{filepath=Filepath1, fileinfo=Fileinfo, mimetypes=Mimetypes1,
243 243 etag_fun=ETagFunction}
244 244 end,
@@ -322,7 +322,21 @@ file_contents(Req, #state{filepath=Filepath,
322 322 fileinfo={ok, #file_info{size=Filesize}}}=State) ->
323 323 {ok, Transport, Socket} = cowboy_req:transport(Req),
324 324 Writefile = content_function(Transport, Socket, Filepath),
325   - {{stream, Filesize, Writefile}, Req, State}.
  325 + % if a data range requested, specify limits
  326 + % NB: so far we handle only one range
  327 + {Headers, _Req1} = cowboy_req:headers(Req),
  328 + Range = cowboy_util:get_ranges(
  329 + proplists:get_value(<<"range">>, Headers, <<>>),
  330 + <<"bytes">>, Filesize),
  331 + case Range of
  332 + % TODO: Start < 0 or End < Start should result in 416 Requested Range Not Satisfiable
  333 + [{Start, End}] when Start >= 0, End >= Start ->
  334 + {{stream, Start, End, Filesize, Writefile}, Req, State};
  335 + [{Start, End} | _T] when Start >= 0, End >= Start ->
  336 + {{stream, Start, End, Filesize, Writefile}, Req, State};
  337 + _ ->
  338 + {{stream, Filesize, Writefile}, Req, State}
  339 + end.
326 340
327 341
328 342 %% @private Return a function writing the contents of a file to a socket.
66 src/cowboy_util.erl
... ... @@ -0,0 +1,66 @@
  1 +-module(cowboy_util).
  2 +
  3 +-export([get_ranges/2, get_ranges/3]).
  4 +-export([parse_range_header/1, parse_range_header/2]).
  5 +
  6 +get_ranges(Header, Name) ->
  7 + get_ranges(Header, Name, 0).
  8 +
  9 +get_ranges(Header, Name, Length) ->
  10 + NameStr = binary_to_list(Name),
  11 + lists:foldr(fun(E, A) ->
  12 + case E of
  13 + {NameStr, bad} ->
  14 + [bad | A];
  15 + {NameStr, Start, End} ->
  16 + [{Start, End} | A];
  17 + _ ->
  18 + A
  19 + end
  20 + end, [], parse_range_header(Header, Length)).
  21 +
  22 +parse_range_header(Header) ->
  23 + parse_range_header(Header, 0).
  24 +
  25 +parse_range_header(Header, Length) when is_binary(Header), is_integer(Length) ->
  26 + MayBeRanges = string:tokens(binary_to_list(Header), "\s;"),
  27 + Ranges = lists:map(fun(E) ->
  28 + case re:run(E, "^([a-z]+)=([0-9-]+)$", [{capture, all, list}]) of
  29 + {match, [_, Name, Value]} ->
  30 + case parse_range(Value) of
  31 + bad ->
  32 + {Name, bad};
  33 + {Start, End} ->
  34 + Start1 = wrap_negative(Start, Length),
  35 + End1 = wrap_negative(End, Length),
  36 + {Name, Start1, End1}
  37 + end;
  38 + _ -> bad
  39 + end
  40 + end, MayBeRanges),
  41 + Ranges;
  42 +
  43 +parse_range_header(undefined, _) ->
  44 + undefined.
  45 +
  46 +% X-Y
  47 +parse_range(Data) ->
  48 + case string:to_integer(Data) of
  49 + {error, _} ->
  50 + bad;
  51 + {Start, ""} ->
  52 + {Start, Start};
  53 + {Start, "-"} ->
  54 + {Start, -1};
  55 + {Start, "-" ++ Data1} ->
  56 + case string:to_integer(Data1) of
  57 + {End, ""} ->
  58 + {Start, End};
  59 + _ ->
  60 + bad
  61 + end
  62 + end.
  63 +
  64 +% X < 0 ? X + Y : X
  65 +wrap_negative(X, L) when X < 0 -> X + L;
  66 +wrap_negative(X, _L) -> X.

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.