From: Eric Wong <e@80x24.org>
To: unicorn-public@bogomips.org
Subject: [RFC] http: TypedData C-API conversion
Date: Sun, 16 Nov 2014 08:32:16 +0000 [thread overview]
Message-ID: <typed-data-c-api@1.9.x> (raw)
This provides some extra type safety if combined with other C
extensions, as well as allowing us to account for memory usage of
the HTTP parser in ObjectSpace.
Note: this means we are finally dropping Ruby 1.8 support as
TypedData requires Ruby 1.9 and later. Future changes will
require Ruby 1.9.2 and later (which is already EOL), but still
in use at some places. This compiles with warnings on Ruby 1.9.2,
but is warning-free on modern Ruby versions.
This also currently leaks memory under 1.9.2-p330 x86_64-linux if
test_memory_leak is enabled in test/unit/test_http_parser.rb
Since 1.9.2 is EOL and 1.9.3+ all work fine (including trunk),
I'm not going to spend more time with this problem.
Also, keep in mind this type of memory leak wouldn't affect unicorn
as we only ever allocate a single parser. This leak would only
affect other (concurrent) HTTP servers using this parser, and only
under Ruby 1.9.2.
---
So with this, I'm strongly considering making unicorn 5 depend
on Ruby 1.9.3+, and not just Ruby 1.9.2+.
ext/unicorn_http/unicorn_http.rl | 35 +++++++++++++++++++++++------------
test/unit/test_http_parser.rb | 10 ++++++++++
2 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl
index 3294357..16741e0 100644
--- a/ext/unicorn_http/unicorn_http.rl
+++ b/ext/unicorn_http/unicorn_http.rl
@@ -417,11 +417,31 @@ post_exec: /* "_out:" also goes here */
assert(hp->offset <= len && "offset longer than length");
}
+static void hp_mark(void *ptr)
+{
+ struct http_parser *hp = ptr;
+
+ rb_gc_mark(hp->buf);
+ rb_gc_mark(hp->env);
+ rb_gc_mark(hp->cont);
+}
+
+static size_t hp_memsize(const void *ptr)
+{
+ return sizeof(struct http_parser);
+}
+
+static const rb_data_type_t hp_type = {
+ "unicorn_http",
+ { hp_mark, (void(*))(void *)-1, hp_memsize, /* reserved */ },
+ /* parent, data, [ flags ] */
+};
+
static struct http_parser *data_get(VALUE self)
{
struct http_parser *hp;
- Data_Get_Struct(self, struct http_parser, hp);
+ TypedData_Get_Struct(self, struct http_parser, &hp_type, hp);
assert(hp && "failed to extract http_parser struct");
return hp;
}
@@ -527,21 +547,12 @@ static void finalize_header(struct http_parser *hp)
rb_hash_aset(hp->env, g_query_string, rb_str_new(NULL, 0));
}
-static void hp_mark(void *ptr)
-{
- struct http_parser *hp = ptr;
-
- rb_gc_mark(hp->buf);
- rb_gc_mark(hp->env);
- rb_gc_mark(hp->cont);
-}
-
static VALUE HttpParser_alloc(VALUE klass)
{
struct http_parser *hp;
- return Data_Make_Struct(klass, struct http_parser, hp_mark, -1, hp);
-}
+ return TypedData_Make_Struct(klass, struct http_parser, &hp_type, hp);
+}
/**
* call-seq:
diff --git a/test/unit/test_http_parser.rb b/test/unit/test_http_parser.rb
index 2251dcf..2785df7 100644
--- a/test/unit/test_http_parser.rb
+++ b/test/unit/test_http_parser.rb
@@ -851,4 +851,14 @@ class HttpParserTest < Test::Unit::TestCase
File.readable?(LINUX_PROC_PID_STATUS) &&
!defined?(RUBY_ENGINE)
+ def test_memsize
+ require 'objspace'
+ if ObjectSpace.respond_to?(:memsize_of)
+ n = ObjectSpace.memsize_of(Unicorn::HttpParser.new)
+ assert_kind_of Integer, n
+ assert_operator n, :<=, 56 # need to update this when 128-bit machines
+ end
+ rescue LoadError
+ # not all Ruby implementations have objspace
+ end
end
--
EW
next reply other threads:[~2014-11-16 8:32 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-16 8:32 Eric Wong [this message]
2014-12-28 7:06 ` [RFC] http: TypedData C-API conversion Eric Wong
2014-12-28 7:17 ` [PATCH] tmpio: drop the "size" method Eric Wong
2015-12-24 18:57 ` [RFC] http: TypedData C-API conversion Eric Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://yhbt.net/unicorn/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=typed-data-c-api@1.9.x \
--to=e@80x24.org \
--cc=unicorn-public@bogomips.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://yhbt.net/unicorn.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).