Permalink
Browse files

Make env() faster.

* Changes the calling convention from $r->env($e) to $e = $r->env;
* Uses a "template" hash so a new env can be set up without a lot of
  re-hashing.
* The template uses placeholders for some common headers.  These "disappear"
  when you iterate through the keys or try to access them directly.
  • Loading branch information...
1 parent be67bb9 commit 926ad448617551a49fd934f6a61c533f8d919b89 @stash committed Sep 19, 2010
Showing with 160 additions and 73 deletions.
  1. +82 −33 Feersum.xs
  2. +5 −7 lib/Feersum.pm
  3. +4 −1 lib/Feersum/Runner.pm
  4. +4 −5 t/01-simple.t
  5. +58 −16 t/03-env-hash.t
  6. +1 −2 t/05-streaming.t
  7. +3 −4 t/06-input.t
  8. +1 −2 t/07-graceful-shutdown.t
  9. +2 −3 t/08-read-timeout.t
View
@@ -115,7 +115,7 @@ typedef struct feer_conn feer_conn_handle; // for typemap
#define dCONN struct feer_conn *c = (struct feer_conn *)w->data
-static SV* feersum_env(pTHX_ struct feer_conn *c, HV *e);
+static HV* feersum_env(pTHX_ struct feer_conn *c);
static void feersum_start_response
(pTHX_ struct feer_conn *c, SV *message, AV *headers, int streaming);
static int feersum_write_whole_body (pTHX_ struct feer_conn *c, SV *body);
@@ -170,6 +170,7 @@ static SV *psgi_serv10, *psgi_serv11, *crlf_sv;
// TODO: make this thread-local if and when there are multiple C threads:
struct ev_loop *feersum_ev_loop = NULL;
+static HV *feersum_tmpl_env = NULL;
INLINE_UNLESS_DEBUG
static struct iomatrix *
@@ -1060,14 +1061,15 @@ needs_decode:
SvCUR_set(sv, decoded-ptr);
}
-static SV*
-feersum_env(pTHX_ struct feer_conn *c, HV *e)
+static void
+feersum_init_tmpl_env(pTHX)
{
- SV **hsv;
- int i,j;
- struct feer_req *r = c->req;
+ HV *e;
+ e = newHV();
+
+ // strlen: 012345678901234567890123456789
- // strlen: 0123456789012345678901
+ // constants
hv_store(e, "psgi.version", 12, newRV((SV*)psgi_ver), 0);
hv_store(e, "psgi.url_scheme", 15, newSVpvn("http",4), 0);
hv_store(e, "psgi.nonblocking", 16, &PL_sv_yes, 0);
@@ -1078,45 +1080,90 @@ feersum_env(pTHX_ struct feer_conn *c, HV *e)
hv_store(e, "psgix.input.buffered", 20, &PL_sv_yes, 0);
hv_store(e, "psgix.output.buffered", 21, &PL_sv_yes, 0);
hv_store(e, "psgix.body.scalar_refs", 22, &PL_sv_yes, 0);
- hv_store(e, "REQUEST_URI", 11, newSVpvn(r->path,r->path_len),0);
- hv_store(e, "REQUEST_METHOD", 14, newSVpvn(r->method,r->method_len),0);
hv_store(e, "SCRIPT_NAME", 11, newSVpvn("",0),0);
+ // placeholders that get defined for every request
+ hv_store(e, "SERVER_PROTOCOL", 15, &PL_sv_undef, 0);
+ hv_store(e, "SERVER_NAME", 11, &PL_sv_undef, 0);
+ hv_store(e, "SERVER_PORT", 11, &PL_sv_undef, 0);
+ hv_store(e, "REQUEST_URI", 11, &PL_sv_undef, 0);
+ hv_store(e, "REQUEST_METHOD", 14, &PL_sv_undef, 0);
+ hv_store(e, "PATH_INFO", 9, &PL_sv_undef, 0);
+
+ // defaults that get changed for some requests
+ hv_store(e, "psgi.input", 10, &PL_sv_undef, 0);
+ hv_store(e, "CONTENT_LENGTH", 14, newSViv(0), 0);
+ hv_store(e, "QUERY_STRING", 12, newSVpvn("",0), 0);
+
+ // anticipated headers
+ hv_store(e, "HTTP_HOST", 9, &PL_sv_placeholder, 0);
+ hv_store(e, "HTTP_USER_AGENT", 15, &PL_sv_placeholder, 0);
+ hv_store(e, "HTTP_ACCEPT", 11, &PL_sv_placeholder, 0);
+ hv_store(e, "HTTP_ACCEPT_LANGUAGE", 20, &PL_sv_placeholder, 0);
+ hv_store(e, "HTTP_ACCEPT_CHARSET", 19, &PL_sv_placeholder, 0);
+ hv_store(e, "HTTP_KEEP_ALIVE", 15, &PL_sv_placeholder, 0);
+ hv_store(e, "HTTP_CONNECTION", 15, &PL_sv_placeholder, 0);
+ hv_store(e, "HTTP_REFERER", 12, &PL_sv_placeholder, 0);
+ hv_store(e, "HTTP_COOKIE", 11, &PL_sv_placeholder, 0);
+ hv_store(e, "HTTP_IF_MODIFIED_SINCE", 22, &PL_sv_placeholder, 0);
+ hv_store(e, "HTTP_IF_NONE_MATCH", 18, &PL_sv_placeholder, 0);
+ hv_store(e, "HTTP_CACHE_CONTROL", 18, &PL_sv_placeholder, 0);
+
+ feersum_tmpl_env = e;
+}
+
+static HV*
+feersum_env(pTHX_ struct feer_conn *c)
+{
+ HV *e;
+ SV **hsv;
+ int i,j;
+ struct feer_req *r = c->req;
+
+ if (!feersum_tmpl_env)
+ feersum_init_tmpl_env(aTHX);
+ e = newHVhv(feersum_tmpl_env);
+
+ trace("generating header (fd %d) %.*s\n",
+ c->fd, r->path_len, r->path);
+
+ SV *path = newSVpvn(r->path, r->path_len);
+ hv_store(e, "SERVER_NAME", 11, newSVsv(feer_server_name), 0);
+ hv_store(e, "SERVER_PORT", 11, newSVsv(feer_server_port), 0);
+ hv_store(e, "REQUEST_URI", 11, path, 0);
+ hv_store(e, "REQUEST_METHOD", 14, newSVpvn(r->method,r->method_len), 0);
hv_store(e, "SERVER_PROTOCOL", 15, (r->minor_version == 1) ?
newSVsv(psgi_serv11) : newSVsv(psgi_serv10), 0);
- SvREFCNT_inc(feer_server_name);
- hv_store(e, "SERVER_NAME", 11, feer_server_name, 0);
- SvREFCNT_inc(feer_server_port);
- hv_store(e, "SERVER_PORT", 11, feer_server_port, 0);
if (c->expected_cl > 0) {
hv_store(e, "CONTENT_LENGTH", 14, newSViv(c->expected_cl), 0);
hv_store(e, "psgi.input", 10, new_feer_conn_handle(c,0), 0);
}
- else {
- hv_store(e, "CONTENT_LENGTH", 14, newSViv(0), 0);
- // TODO: make this a valid, but always empty stream for PSGI mode?
- hv_store(e, "psgi.input", 10, &PL_sv_undef, 0);
+ else if (request_cb_is_psgi) {
+ // TODO: make psgi.input a valid, but always empty stream for PSGI mode?
}
{
const char *qpos = r->path;
SV *pinfo, *qstr;
- while (*qpos != '?' && qpos < r->path + r->path_len) {
+
+ // rather than memchr, for speed:
+ while (*qpos != '?' && qpos < r->path + r->path_len)
qpos++;
- }
+
if (*qpos == '?') {
pinfo = newSVpvn(r->path, (qpos - r->path));
qpos++;
qstr = newSVpvn(qpos, r->path_len - (qpos - r->path));
}
else {
- pinfo = newSVpvn(r->path, r->path_len);
- qstr = newSVpvn("",0);
+ pinfo = newSVsv(path);
+ qstr = NULL; // use template default
}
uri_decode_sv(pinfo);
hv_store(e, "PATH_INFO", 9, pinfo, 0);
- hv_store(e, "QUERY_STRING", 12, qstr, 0);
+ if (qstr != NULL) // hv template defaults QUERY_STRING to empty
+ hv_store(e, "QUERY_STRING", 12, qstr, 0);
}
SV *val = NULL;
@@ -1149,8 +1196,8 @@ feersum_env(pTHX_ struct feer_conn *c, HV *e)
}
SV **val = hv_fetch(e, kbuf, klen, 1);
- trace("adding header to env %.*s: %.*s\n",
- klen, kbuf, hdr->value_len, hdr->value);
+ trace("adding header to env (fd %d) %.*s: %.*s\n",
+ c->fd, klen, kbuf, hdr->value_len, hdr->value);
assert(val != NULL); // "fetch is store" flag should ensure this
if (SvPOK(*val)) {
@@ -1165,8 +1212,9 @@ feersum_env(pTHX_ struct feer_conn *c, HV *e)
}
}
+ // TODO: free c->req now that all the keys are copied to scalars?
Safefree(kbuf);
- return (SV*)e;
+ return e;
}
static void
@@ -1370,11 +1418,10 @@ call_request_callback (struct feer_conn *c)
PUSHMARK(SP);
if (request_cb_is_psgi) {
- SV *env = feersum_env(aTHX_ c,newHV());
+ HV *env = feersum_env(aTHX_ c);
// SV *conn_sv = feer_conn_2sv(c);
// hv_store((HV*)env, "psgix.feersum", 13, conn_sv, 0);
- env = sv_2mortal(newRV_noinc(env));
- XPUSHs(env);
+ XPUSHs(sv_2mortal(newRV_noinc((SV*)env)));
flags = G_EVAL|G_SCALAR;
}
else {
@@ -1774,11 +1821,13 @@ _handle (struct feer_conn *c)
OUTPUT:
RETVAL
-void
-env (struct feer_conn *c, HV *e)
- PROTOTYPE: $\%
- PPCODE:
- feersum_env(aTHX_ c,e);
+SV *
+env (struct feer_conn *c)
+ PROTOTYPE: $
+ CODE:
+ RETVAL = newRV_noinc((SV*)feersum_env(aTHX_ c));
+ OUTPUT:
+ RETVAL
int
fileno (struct feer_conn *c)
View
@@ -5,7 +5,7 @@ use warnings;
use EV ();
use Carp ();
-our $VERSION = '0.90';
+our $VERSION = '0.91';
require XSLoader;
XSLoader::load('Feersum', $VERSION);
@@ -143,8 +143,7 @@ the PSGI env hash).
A PSGI-like environment hash is easy to obtain.
my $req = shift;
- my %env;
- $req->env(\%env);
+ my $env = $req->env();
Currently POST/PUT does not stream input, but read() can be called on
C<psgi.input> to get the body (which has been buffered up before the request
@@ -154,12 +153,11 @@ the arrival of more data. (The C<psgix.input.buffered> env var is set to
reflect this).
my $req = shift;
- my %env;
- $req->env(\%env);
+ my $env = $req->env();
if ($req->{REQUEST_METHOD} eq 'POST') {
my $body = '';
- my $r = delete $env{'psgi.input'};
- $r->read($body, $env{CONTENT_LENGTH});
+ my $r = delete $env->{'psgi.input'};
+ $r->read($body, $env->{CONTENT_LENGTH});
# optional: choose to stop receiving further input:
# $r->close();
}
View
@@ -37,7 +37,10 @@ sub _prepare {
sub run {
my $self = shift;
$self->_prepare();
- $self->{endjinn}->request_handler(shift || delete $self->{app});
+ my $rh = shift || delete $self->{app};
+ die "not a code ref" unless ref($rh) eq 'CODE';
+ $self->{endjinn}->request_handler($rh);
+ undef $rh;
EV::loop;
}
View
@@ -41,11 +41,10 @@ $cb = sub {
isa_ok $r, 'Feersum::Connection', 'got an object!';
# use Devel::Peek();
# Devel::Peek::Dump($r);
- my %env;
- $r->env(\%env);
- ok %env, "got env";
- is $env{HTTP_USER_AGENT}, 'FeersumSimpleClient/1.0', 'got a ua!';
- my $utf8 = exists $env{HTTP_X_UNICODE_PLEASE};
+ my $env = $r->env();
+ ok $env, "got env";
+ is $env->{HTTP_USER_AGENT}, 'FeersumSimpleClient/1.0', 'got a ua!';
+ my $utf8 = exists $env->{HTTP_X_UNICODE_PLEASE};
eval {
$r->send_response("200 OK", [
'Content-Type' => 'text/plain'.($utf8 ? '; charset=UTF-8' : ''),
View
@@ -1,7 +1,7 @@
#!perl
use warnings;
use strict;
-use Test::More tests => 61;
+use Test::More tests => 97;
use Test::Exception;
use utf8;
use lib 't'; use Utils;
@@ -24,11 +24,12 @@ my $evh = Feersum->new();
$evh->request_handler(sub {
my $r = shift;
- isa_ok $r, 'Feersum::Connection', 'got an object!';
- ok ($r->fileno && !blessed($r->fileno));
- my $env = {};
- $r->env($env);
- ok $env && ref($env) eq 'HASH';
+ isa_ok $r, 'Feersum::Connection', 'connection';
+ my $env = $r->env();
+ ok $env && ref($env) eq 'HASH', "env hash";
+
+ my $tn = $env->{HTTP_X_TEST_NUM} || 0;
+ ok $tn, "got a test number header $tn";
is_deeply $env->{'psgi.version'}, [1,0], 'got psgi.version';
is $env->{'psgi.url_scheme'}, "http", 'got psgi.url_scheme';
@@ -47,23 +48,33 @@ $evh->request_handler(sub {
is $env->{CONTENT_LENGTH}, 0, "got zero C-L";
ok !exists $env->{HTTP_CONTENT_LENGTH}, "no duplicate C-L header";
- ok $env->{HTTP_X_TEST_NUM}, "got a test number header";
- if ($env->{HTTP_X_TEST_NUM} == 1) {
+ if ($tn == 1) {
like $env->{HTTP_REFERER}, qr/wrong/, "got the Referer";
is $env->{QUERY_STRING}, 'blar', "got query string";
is $env->{PATH_INFO}, '/what is wrong?', "got decoded path info string";
is $env->{REQUEST_URI}, '/what%20is%20wrong%3f?blar', "got full URI string";
}
- else {
- like $env->{HTTP_REFERER}, qr/good/, "got the AE Referer";
+ elsif ($tn == 2) {
+ like $env->{HTTP_REFERER}, qr/good/, "got a Referer";
is $env->{QUERY_STRING}, 'dlux=sonice', "got query string";
is $env->{PATH_INFO}, '/what% is good?%2', "got decoded path info string";
is $env->{REQUEST_URI}, '/what%%20is%20good%3F%2?dlux=sonice', "got full URI string";
}
+ elsif ($tn == 3) {
+ like $env->{HTTP_REFERER}, qr/ugly/, "got a Referer";
+ is $env->{QUERY_STRING}, '', "got query string";
+ is $env->{PATH_INFO}, '/no query', "got decoded path info string";
+ is $env->{REQUEST_URI}, '/no%20query', "got full URI string";
+ }
is $env->{SERVER_NAME}, '127.0.0.1', "got server name";
is $env->{SERVER_PORT}, $port, "got server port";
+ ok !exists $env->{HTTP_ACCEPT_CHARSET},
+ "spot check that a placeholder Accept-Charset isn't there";
+ ok !exists $env->{HTTP_ACCEPT_LANGUAGE},
+ "spot check that a placeholder Accept-Language isn't there";
+
lives_ok {
$r->send_response("200 OK", [
'Content-Type' => 'text/plain; charset=UTF-8',
@@ -83,15 +94,15 @@ my $w = simple_client GET => "/what%20is%20wrong%3f?blar",
timeout => 3,
sub {
my ($body, $headers) = @_;
- is $headers->{Status}, 200, "client got 200";
+ is $headers->{Status}, 200, "client 1 got 200";
is $headers->{'content-type'}, 'text/plain; charset=UTF-8';
$body = Encode::decode_utf8($body) unless Encode::is_utf8($body);
is $headers->{'content-length'}, bytes::length($body),
- 'content-length was calculated correctly';
+ 'client 1 content-length was calculated correctly';
- is $body, "Oh Hai 1\n", 'got expected body';
+ is $body, "Oh Hai 1\n", 'client 1 expected body';
$cv->end;
};
@@ -101,15 +112,46 @@ my $w2 = simple_client GET => "/what%%20is%20good%3F%2?dlux=sonice",
timeout => 3,
sub {
my ($body, $headers) = @_;
- is $headers->{Status}, 200, "client got 200";
+ is $headers->{Status}, 200, "client 2 got 200";
is $headers->{'content-type'}, 'text/plain; charset=UTF-8';
$body = Encode::decode_utf8($body) unless Encode::is_utf8($body);
is $headers->{'content-length'}, bytes::length($body),
- 'content-length was calculated correctly';
+ 'client 2 content-length was calculated correctly';
+
+ is $body, "Oh Hai 2\n", 'client 2 expected body';
+ $cv->end;
+};
+
+$cv->begin;
+my $w3 = simple_client GET => "/no%20query",
+ headers => {'x-test-num' => 3, 'Referer' => 'ugly'},
+ timeout => 3,
+sub {
+ my ($body, $headers) = @_;
+ is $headers->{Status}, 200, "client 3 got 200";
+ is $headers->{'content-type'}, 'text/plain; charset=UTF-8';
- is $body, "Oh Hai 2\n", 'got expected body';
+ $body = Encode::decode_utf8($body) unless Encode::is_utf8($body);
+
+ is $headers->{'content-length'}, bytes::length($body),
+ 'client 3 content-length was calculated correctly';
+
+ is $body, "Oh Hai 3\n", 'client 3 expected body';
+ $cv->end;
+};
+
+$cv->begin;
+my $w4 = simple_client GET => "/no spaces allowed",
+ headers => {'x-test-num' => 4, 'Referer' => 'ugly'},
+ timeout => 3,
+sub {
+ my ($body, $headers) = @_;
+ is $headers->{Status}, 400, 'client 4 Bad Request';
+ is $headers->{Reason}, "Bad Request";
+ is $headers->{'content-type'}, 'text/plain';
+ is $body, "Malformed request.\n", 'client 4 expected error';
$cv->end;
};
Oops, something went wrong.

0 comments on commit 926ad44

Please sign in to comment.