]> git.wincent.com - wikitext.git/commitdiff
Let the compiler decide when to inline
authorWincent Colaiuta <win@wincent.com>
Fri, 29 Feb 2008 12:12:23 +0000 (13:12 +0100)
committerWincent Colaiuta <win@wincent.com>
Fri, 29 Feb 2008 12:12:23 +0000 (13:12 +0100)
This change was initially prompted because I was getting duplicate
symbol errors when linking for functions which should have been
inlined on RHEL 5.1.

My first trouble-shooting technique was disabling inlining to confirm
that the linker errors went away (they did) but it turns out that there
was a secondary benefit: the compiler is actually much smarter than me,
because when I leave things up to the compiler instead of explicitly
requesting inlining, the spec suite runs 10 to 20% faster. There is
quite a bit of Ruby/RSpec overhead in the spec suite, so the speed
improvement of the parser itself is probably significantly more.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
ext/ary.c
ext/ary.h
ext/parser.c
ext/str.c
ext/str.h

index 4f3885d59e899b8145140a23931ee561213c689e..4bc6655fbfd55fcbf795590bd3382c11f6c52371 100644 (file)
--- a/ext/ary.c
+++ b/ext/ary.c
 
 #include "ary.h"
 
+ary_t *ary_new(void)
+{
+    ary_t *ary      = ALLOC_N(ary_t, 1);
+    ary->count      = 0;
+    ary->max        = DEFAULT_ENTRY_COUNT;
+    ary->entries    = ALLOC_N(int, DEFAULT_ENTRY_COUNT);
+    return ary;
+}
+
+int ary_entry(ary_t *ary, int idx)
+{
+    if (idx < 0)
+        idx = ary->count + idx;
+    return (idx >= 0 && ary->count > idx) ? ary->entries[idx] : INT_MAX;
+}
+
+void ary_clear(ary_t *ary)
+{
+    ary->count = 0;
+}
+
+int ary_pop(ary_t *ary)
+{
+    if (ary->count > 0)
+    {
+        ary->count--;
+        return 1;
+    }
+    return 0;
+}
+
+void ary_push(ary_t *ary, int val)
+{
+    if (ary->count == ary->max)
+    {
+        ary->max += DEFAULT_ENTRY_COUNT;
+        REALLOC_N(ary->entries, int, ary->max);
+    }
+    ary->entries[ary->count] = val;
+    ary->count++;
+}
+
+int ary_includes(ary_t *ary, int val)
+{
+    for (int i = 0, max = ary->count; i < max; i++)
+    {
+        if (ary->entries[i] == val)
+            return 1;
+    }
+    return 0;
+}
+
+int ary_count(ary_t *ary, int item)
+{
+    int count = 0;
+    for (int i = 0, max = ary->count; i < max; i++)
+    {
+        if (ary->entries[i] == item)
+            count++;
+    }
+    return count;
+}
+
 void ary_free(ary_t *ary)
 {
     free(ary->entries);
index 0c415f65a1c835e4bc942951fbc44b46abf9e1c7..bbb41fa25c6762df50997700f2d94d583dfe4f26 100644 (file)
--- a/ext/ary.h
+++ b/ext/ary.h
@@ -30,71 +30,16 @@ typedef struct
 // A variable named name is placed on the C stack to prevent the structure from being prematurely collected.
 #define GC_WRAP_ARY(ptr, name) volatile VALUE name __attribute__((unused)) = Data_Wrap_Struct(rb_cObject, 0, ary_free, ptr)
 
-inline ary_t *ary_new(void)
-{
-    ary_t *ary      = ALLOC_N(ary_t, 1);
-    ary->count      = 0;
-    ary->max        = DEFAULT_ENTRY_COUNT;
-    ary->entries    = ALLOC_N(int, DEFAULT_ENTRY_COUNT);
-    return ary;
-}
-
-// this method not inlined so its address can be passed to the Data_Wrap_Struct function.
-void ary_free(ary_t *ary);
-
-inline int ary_entry(ary_t *ary, int idx)
-{
-    if (idx < 0)
-        idx = ary->count + idx;
-    return (idx >= 0 && ary->count > idx) ? ary->entries[idx] : INT_MAX;
-}
-
-inline void ary_clear(ary_t *ary)
-{
-    ary->count = 0;
-}
-
-inline int ary_pop(ary_t *ary)
-{
-    if (ary->count > 0)
-    {
-        ary->count--;
-        return 1;
-    }
-    return 0;
-}
-
-inline void ary_push(ary_t *ary, int val)
-{
-    if (ary->count == ary->max)
-    {
-        ary->max += DEFAULT_ENTRY_COUNT;
-        REALLOC_N(ary->entries, int, ary->max);
-    }
-    ary->entries[ary->count] = val;
-    ary->count++;
-}
-
-inline int ary_includes(ary_t *ary, int val)
-{
-    for (int i = 0, max = ary->count; i < max; i++)
-    {
-        if (ary->entries[i] == val)
-            return 1;
-    }
-    return 0;
-}
+ary_t *ary_new(void);
+int ary_entry(ary_t *ary, int idx);
+void ary_clear(ary_t *ary);
+int ary_pop(ary_t *ary);
+void ary_push(ary_t *ary, int val);
+int ary_includes(ary_t *ary, int val);
 
 // returns a count indicating the number of times the value appears in the collection
 // refactored from _Wikitext_count()
-inline int ary_count(ary_t *ary, int item)
-{
-    int count = 0;
-    for (int i = 0, max = ary->count; i < max; i++)
-    {
-        if (ary->entries[i] == item)
-            count++;
-    }
-    return count;
-}
+int ary_count(ary_t *ary, int item);
 
+// this method not inlined so its address can be passed to the Data_Wrap_Struct function.
+void ary_free(ary_t *ary);
index 8ed2493674704b253adca96ac43a67c1b5de6058..03b14890ddc862021c35485744211d7b23777caf 100644 (file)
@@ -158,7 +158,7 @@ VALUE Wikitext_parser_benchmarking_tokenize(VALUE self, VALUE string)
 }
 
 // we downcase "in place", overwriting the original contents of the buffer and returning the same string
-inline VALUE _Wikitext_downcase(VALUE string)
+VALUE _Wikitext_downcase(VALUE string)
 {
     char *ptr   = RSTRING_PTR(string);
     long len    = RSTRING_LEN(string);
@@ -170,7 +170,7 @@ inline VALUE _Wikitext_downcase(VALUE string)
     return string;
 }
 
-inline VALUE _Wikitext_hyperlink(VALUE link_prefix, VALUE link_target, VALUE link_text, VALUE link_class)
+VALUE _Wikitext_hyperlink(VALUE link_prefix, VALUE link_target, VALUE link_text, VALUE link_class)
 {
     VALUE string = rb_str_new(a_start, sizeof(a_start) - 1);        // <a href="
     if (!NIL_P(link_prefix))
@@ -187,7 +187,7 @@ inline VALUE _Wikitext_hyperlink(VALUE link_prefix, VALUE link_target, VALUE lin
     return string;
 }
 
-inline void _Wikitext_append_img(parser_t *parser, char *token_ptr, int token_len)
+void _Wikitext_append_img(parser_t *parser, char *token_ptr, int token_len)
 {
     rb_str_cat(parser->output, img_start, sizeof(img_start) - 1);   // <img src="
     if (!NIL_P(parser->img_prefix))
@@ -201,7 +201,7 @@ inline void _Wikitext_append_img(parser_t *parser, char *token_ptr, int token_le
 // will emit indentation only if we are about to emit any of:
 //      <blockquote>, <p>, <ul>, <ol>, <li>, <h1> etc, <pre>
 // each time we enter one of those spans must ++ the indentation level
-inline void _Wikitext_indent(parser_t *parser)
+void _Wikitext_indent(parser_t *parser)
 {
     int space_count = parser->current_indent + parser->base_indent;
     if (space_count > 0)
@@ -220,7 +220,7 @@ inline void _Wikitext_indent(parser_t *parser)
     parser->current_indent += 2;
 }
 
-inline void _Wikitext_dedent(parser_t *parser, VALUE emit)
+void _Wikitext_dedent(parser_t *parser, VALUE emit)
 {
     parser->current_indent -= 2;
     if (emit != Qtrue)
@@ -393,7 +393,7 @@ void _Wikitext_pop_from_stack_up_to(parser_t *parser, VALUE target, int item, VA
     } while (continue_looping);
 }
 
-inline void _Wikitext_start_para_if_necessary(parser_t *parser)
+void _Wikitext_start_para_if_necessary(parser_t *parser)
 {
     if (!NIL_P(parser->capture))    // we don't do anything if in capturing mode
         return;
@@ -442,7 +442,7 @@ inline void _Wikitext_start_para_if_necessary(parser_t *parser)
 // on the line scope.
 // Luckily, BLOCKQUOTE_START tokens can only appear at the start of the scope array, so we can check for them first before
 // entering the for loop.
-inline void _Wikitext_pop_excess_elements(parser_t *parser)
+void _Wikitext_pop_excess_elements(parser_t *parser)
 {
     if (!NIL_P(parser->capture)) // we don't pop anything if in capturing mode
         return;
@@ -471,7 +471,7 @@ inline void _Wikitext_pop_excess_elements(parser_t *parser)
 // the number of bytes in the UTF-8 character (between 1 and 4) is returned by reference in width_out
 // raises a RangeError if the supplied character is invalid UTF-8
 // (in which case it also frees the block of memory indicated by dest_ptr if it is non-NULL)
-inline uint32_t _Wikitext_utf8_to_utf32(char *src, char *end, long *width_out, void *dest_ptr)
+uint32_t _Wikitext_utf8_to_utf32(char *src, char *end, long *width_out, void *dest_ptr)
 {
     uint32_t dest;
     if ((unsigned char)src[0] <= 0x7f)                      // ASCII
@@ -515,7 +515,7 @@ inline uint32_t _Wikitext_utf8_to_utf32(char *src, char *end, long *width_out, v
     return dest;
 }
 
-inline VALUE _Wikitext_utf32_char_to_entity(uint32_t character)
+VALUE _Wikitext_utf32_char_to_entity(uint32_t character)
 {
     // TODO: consider special casing some entities (ie. quot, amp, lt, gt etc)?
     char hex_string[8]  = { '&', '#', 'x', 0, 0, 0, 0, ';' };
@@ -530,7 +530,7 @@ inline VALUE _Wikitext_utf32_char_to_entity(uint32_t character)
     return rb_str_new((const char *)hex_string, sizeof(hex_string));
 }
 
-inline VALUE _Wikitext_parser_trim_link_target(VALUE string)
+VALUE _Wikitext_parser_trim_link_target(VALUE string)
 {
     string              = StringValue(string);
     char    *src        = RSTRING_PTR(string);
@@ -560,7 +560,7 @@ inline VALUE _Wikitext_parser_trim_link_target(VALUE string)
 // - QUOT and AMP characters converted to named entities
 // - if rollback is Qtrue, there is no special treatment of spaces
 // - if rollback is Qfalse, leading and trailing whitespace trimmed if trimmed
-inline VALUE _Wikitext_parser_sanitize_link_target(parser_t *parser, VALUE rollback)
+VALUE _Wikitext_parser_sanitize_link_target(parser_t *parser, VALUE rollback)
 {
     VALUE string        = StringValue(parser->link_target); // raises if string is nil or doesn't quack like a string
     char    *src        = RSTRING_PTR(string);
@@ -667,7 +667,7 @@ VALUE Wikitext_parser_sanitize_link_target(VALUE self, VALUE string)
 //         thing. [[Foo]] was...
 // this is also where we check treat_slash_as_special is true and act accordingly
 // basically any link target matching /\A[a-z]+\/\d+\z/ is flagged as special
-inline static void _Wikitext_parser_encode_link_target(parser_t *parser)
+static void _Wikitext_parser_encode_link_target(parser_t *parser)
 {
     VALUE in                = StringValue(parser->link_target);
     char        *input      = RSTRING_PTR(in);
@@ -784,8 +784,7 @@ VALUE Wikitext_parser_encode_special_link_target(VALUE self, VALUE in)
     return parser.link_target;
 }
 
-// not sure whether these rollback functions should be inline: could refactor them into a single non-inlined function
-inline void _Wikitext_rollback_failed_link(parser_t *parser)
+void _Wikitext_rollback_failed_link(parser_t *parser)
 {
     if (!IN(LINK_START))
         return; // nothing to do!
@@ -808,7 +807,7 @@ inline void _Wikitext_rollback_failed_link(parser_t *parser)
     parser->link_text   = Qnil;
 }
 
-inline void _Wikitext_rollback_failed_external_link(parser_t *parser)
+void _Wikitext_rollback_failed_external_link(parser_t *parser)
 {
     if (!IN(EXT_LINK_START))
         return; // nothing to do!
index 34f3eef30abf0aeef218cd8ca025f29074a7972f..f08529b6b9789f5d10dd5ca6ceeb1cc30a5e767a 100644 (file)
--- a/ext/str.c
+++ b/ext/str.c
 
 #include "str.h"
 
+str_t *str_new(void)
+{
+    str_t *str      = ALLOC_N(str_t, 1);
+    str->ptr        = NULL;
+    str->len        = 0;
+    str->capacity   = 0;
+    return str;
+}
+
+str_t *str_new_size(long len)
+{
+    str_t *str      = ALLOC_N(str_t, 1);
+    str->ptr        = ALLOC_N(char, len);
+    str->len        = 0;
+    str->capacity   = len;
+    return str;
+}
+
+str_t *str_new_copy(char *src, long len)
+{
+    str_t *str      = ALLOC_N(str_t, 1);
+    str->ptr        = ALLOC_N(char, len);
+    memcpy(str->ptr, src, len);
+    str->len        = len;
+    str->capacity   = len;
+    return str;
+}
+
+str_t *str_new_no_copy(char *src, long len)
+{
+    str_t *str      = ALLOC_N(str_t, 1);
+    str->ptr        = src;
+    str->len        = len;
+    str->capacity   = len;
+    return str;
+}
+
+str_t *str_new_from_string(VALUE string)
+{
+    string = StringValue(string);
+    return str_new_copy(RSTRING_PTR(string), RSTRING_LEN(string));
+}
+
+VALUE string_from_str(str_t *str)
+{
+    return rb_str_new(str->ptr, str->len);
+}
+
+void str_grow(str_t *str, long len)
+{
+    if (str->capacity < len)
+    {
+        if (str->ptr)
+            REALLOC_N(str->ptr, char, len);
+        else
+            str->ptr = ALLOC_N(char, len);
+        str->capacity = len;
+    }
+}
+
+void str_append(str_t *str, char *src, long len)
+{
+    long new_len = str->len + len;
+    if (str->capacity < new_len)
+    {
+        if (str->ptr)
+            REALLOC_N(str->ptr, char, new_len);
+        else
+            str->ptr = ALLOC_N(char, new_len);
+        str->capacity = new_len;
+    }
+    memcpy(str->ptr + str->len, src, len);
+    str->len = new_len;
+}
+
+void str_append_str(str_t *str, str_t *other)
+{
+    str_append(str, other->ptr, other->len);
+}
+
+void str_swap(str_t **a, str_t **b)
+{
+    str_t *c;
+    c = *a;
+    *a = *b;
+    *b = c;
+}
+
+void str_clear(str_t *str)
+{
+    str->len = 0;
+}
+
 void str_free(str_t *str)
 {
     if (str->ptr)
index 4c2971df924c9a25663269cc322d37bcfcdcb0a8..7c7446fd1864c7bb2e7424b4cdae2c2f88e1abe9 100644 (file)
--- a/ext/str.h
+++ b/ext/str.h
@@ -26,110 +26,38 @@ typedef struct
 #define GC_WRAP_STR(ptr, name) volatile VALUE name __attribute__((unused)) = Data_Wrap_Struct(rb_cObject, 0, str_free, ptr)
 
 // create a new, empty string struct
-inline str_t *str_new(void)
-{
-    str_t *str      = ALLOC_N(str_t, 1);
-    str->ptr        = NULL;
-    str->len        = 0;
-    str->capacity   = 0;
-    return str;
-}
+str_t *str_new(void);
 
 // create a new, empty string struct with capacity len
-inline str_t *str_new_size(long len)
-{
-    str_t *str      = ALLOC_N(str_t, 1);
-    str->ptr        = ALLOC_N(char, len);
-    str->len        = 0;
-    str->capacity   = len;
-    return str;
-}
+str_t *str_new_size(long len);
 
 // create a new string struct and initialize it with a copy of the buffer of length len pointed to by src
-inline str_t *str_new_copy(char *src, long len)
-{
-    str_t *str      = ALLOC_N(str_t, 1);
-    str->ptr        = ALLOC_N(char, len);
-    memcpy(str->ptr, src, len);
-    str->len        = len;
-    str->capacity   = len;
-    return str;
-}
+str_t *str_new_copy(char *src, long len);
 
 // create a new string struct and initialize it with the buffer of length len pointed to by src
 // no copy is made; the struct takes ownership of the buffer and will free it when the struct is disposed of
-inline str_t *str_new_no_copy(char *src, long len)
-{
-    str_t *str      = ALLOC_N(str_t, 1);
-    str->ptr        = src;
-    str->len        = len;
-    str->capacity   = len;
-    return str;
-}
+str_t *str_new_no_copy(char *src, long len);
 
 // convenience method for testing
-inline str_t *str_new_from_string(VALUE string)
-{
-    string = StringValue(string);
-    return str_new_copy(RSTRING_PTR(string), RSTRING_LEN(string));
-}
+str_t *str_new_from_string(VALUE string);
 
 // convenience method for testing
-inline VALUE string_from_str(str_t *str)
-{
-    return rb_str_new(str->ptr, str->len);
-}
+VALUE string_from_str(str_t *str);
 
 // grows a string's capacity to the specified length
-inline void str_grow(str_t *str, long len)
-{
-    if (str->capacity < len)
-    {
-        if (str->ptr)
-            REALLOC_N(str->ptr, char, len);
-        else
-            str->ptr = ALLOC_N(char, len);
-        str->capacity = len;
-    }
-}
+void str_grow(str_t *str, long len);
 
-inline void str_append(str_t *str, char *src, long len)
-{
-    long new_len = str->len + len;
-    if (str->capacity < new_len)
-    {
-        if (str->ptr)
-            REALLOC_N(str->ptr, char, new_len);
-        else
-            str->ptr = ALLOC_N(char, new_len);
-        str->capacity = new_len;
-    }
-    memcpy(str->ptr + str->len, src, len);
-    str->len = new_len;
-}
+void str_append(str_t *str, char *src, long len);
 
 // appends the "other" string struct onto str
-inline void str_append_str(str_t *str, str_t *other)
-{
-    str_append(str, other->ptr, other->len);
-}
+void str_append_str(str_t *str, str_t *other);
 
 // this is a temporary convenience measure
 // later on if I develop in-place variants of some functions this won't be needed
-inline void str_swap(str_t **a, str_t **b)
-{
-    str_t *c;
-    c = *a;
-    *a = *b;
-    *b = c;
-}
+void str_swap(str_t **a, str_t **b);
 
 // don't actually free the memory yet
 // this makes str structs very useful when reusing buffers because it avoids reallocation
-inline void str_clear(str_t *str)
-{
-    str->len = 0;
-}
+void str_clear(str_t *str);
 
-// this method not inlined so its address can be passed to the Data_Wrap_Struct function.
 void str_free(str_t *str);