From 8e4733c46ede2da806bb83c056f83a53fd1999b7 Mon Sep 17 00:00:00 2001 From: Dylan Thacker-Smith Date: Mon, 28 Sep 2020 20:25:33 -0400 Subject: [PATCH 1/3] Refactor variable compilation to allow evaluation without write --- ext/liquid_c/block.c | 3 +-- ext/liquid_c/variable.c | 20 ++++++++++++-------- ext/liquid_c/variable.h | 4 ++-- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/ext/liquid_c/block.c b/ext/liquid_c/block.c index c3d51da5..bf51ef00 100644 --- a/ext/liquid_c/block.c +++ b/ext/liquid_c/block.c @@ -144,9 +144,8 @@ static tag_markup_t internal_block_body_parse(block_body_t *body, parse_context_ .markup_end = token.str_trimmed + token.len_trimmed, .body = body, .parse_context = parse_context->ruby_obj, - .line_number = token_start_line_number, }; - internal_variable_parse(&parse_args); + internal_variable_compile(&parse_args, token_start_line_number); render_score_increment += 1; body->blank = false; break; diff --git a/ext/liquid_c/variable.c b/ext/liquid_c/variable.c index 71bd5e8f..057c20fc 100644 --- a/ext/liquid_c/variable.c +++ b/ext/liquid_c/variable.c @@ -13,10 +13,10 @@ static VALUE try_variable_strict_parse(VALUE uncast_args) init_parser(&p, parse_args->markup, parse_args->markup_end); vm_assembler_t *code = &parse_args->body->code; - if (p.cur.type == TOKEN_EOS) + if (p.cur.type == TOKEN_EOS) { + vm_assembler_add_push_nil(code); return Qnil; - - vm_assembler_add_render_variable_rescue(code, parse_args->line_number); + } parse_and_compile_expression(&p, code); @@ -70,8 +70,6 @@ static VALUE try_variable_strict_parse(VALUE uncast_args) vm_assembler_add_filter(code, filter_name, arg_count); } - vm_assembler_add_pop_write(code); - parser_must_consume(&p, TOKEN_EOS); return Qnil; @@ -106,17 +104,15 @@ static VALUE variable_strict_parse_rescue(VALUE uncast_args, VALUE exception) // lax parse code->protected_stack_size = code->stack_size; - vm_assembler_add_render_variable_rescue(code, parse_args->line_number); rb_funcall(variable_obj, id_compile_evaluate, 1, parse_args->body->obj); if (code->stack_size != code->protected_stack_size + 1) { rb_raise(rb_eRuntimeError, "Liquid::Variable#compile_evaluate didn't leave exactly 1 new element on the stack"); } - vm_assembler_add_pop_write(code); return Qnil; } -void internal_variable_parse(variable_parse_args_t *parse_args) +void internal_variable_compile_evaluate(variable_parse_args_t *parse_args) { vm_assembler_t *code = &parse_args->body->code; variable_strict_parse_rescue_t rescue_args = { @@ -128,6 +124,14 @@ void internal_variable_parse(variable_parse_args_t *parse_args) rb_rescue(try_variable_strict_parse, (VALUE)parse_args, variable_strict_parse_rescue, (VALUE)&rescue_args); } +void internal_variable_compile(variable_parse_args_t *parse_args, unsigned int line_number) +{ + vm_assembler_t *code = &parse_args->body->code; + vm_assembler_add_render_variable_rescue(code, line_number); + internal_variable_compile_evaluate(parse_args); + vm_assembler_add_pop_write(code); +} + void init_liquid_variable(void) { id_rescue_strict_parse_syntax_error = rb_intern("rescue_strict_parse_syntax_error"); diff --git a/ext/liquid_c/variable.h b/ext/liquid_c/variable.h index 62c548f4..df9c3c13 100644 --- a/ext/liquid_c/variable.h +++ b/ext/liquid_c/variable.h @@ -9,11 +9,11 @@ typedef struct variable_parse_args { const char *markup_end; block_body_t *body; VALUE parse_context; - unsigned int line_number; } variable_parse_args_t; void init_liquid_variable(void); -void internal_variable_parse(variable_parse_args_t *parse_args); +void internal_variable_compile(variable_parse_args_t *parse_args, unsigned int line_number); +void internal_variable_compile_evaluate(variable_parse_args_t *parse_args); #endif From 54a2d72b7cc174754757981d12391a5b5c88cf0e Mon Sep 17 00:00:00 2001 From: Dylan Thacker-Smith Date: Thu, 29 Oct 2020 11:48:18 -0400 Subject: [PATCH 2/3] Refactor variable compilation to allow compilation to an expression object --- ext/liquid_c/block.c | 3 ++- ext/liquid_c/variable.c | 10 +++++----- ext/liquid_c/variable.h | 3 ++- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/ext/liquid_c/block.c b/ext/liquid_c/block.c index bf51ef00..eeed694b 100644 --- a/ext/liquid_c/block.c +++ b/ext/liquid_c/block.c @@ -142,7 +142,8 @@ static tag_markup_t internal_block_body_parse(block_body_t *body, parse_context_ variable_parse_args_t parse_args = { .markup = token.str_trimmed, .markup_end = token.str_trimmed + token.len_trimmed, - .body = body, + .code = &body->code, + .code_obj = body->obj, .parse_context = parse_context->ruby_obj, }; internal_variable_compile(&parse_args, token_start_line_number); diff --git a/ext/liquid_c/variable.c b/ext/liquid_c/variable.c index 057c20fc..40bb978b 100644 --- a/ext/liquid_c/variable.c +++ b/ext/liquid_c/variable.c @@ -11,7 +11,7 @@ static VALUE try_variable_strict_parse(VALUE uncast_args) variable_parse_args_t *parse_args = (void *)uncast_args; parser_t p; init_parser(&p, parse_args->markup, parse_args->markup_end); - vm_assembler_t *code = &parse_args->body->code; + vm_assembler_t *code = parse_args->code; if (p.cur.type == TOKEN_EOS) { vm_assembler_add_push_nil(code); @@ -86,7 +86,7 @@ static VALUE variable_strict_parse_rescue(VALUE uncast_args, VALUE exception) { variable_strict_parse_rescue_t *rescue_args = (void *)uncast_args; variable_parse_args_t *parse_args = rescue_args->parse_args; - vm_assembler_t *code = &parse_args->body->code; + vm_assembler_t *code = parse_args->code; // undo partial strict parse code->instructions.data_end = code->instructions.data + rescue_args->instructions_size; @@ -104,7 +104,7 @@ static VALUE variable_strict_parse_rescue(VALUE uncast_args, VALUE exception) // lax parse code->protected_stack_size = code->stack_size; - rb_funcall(variable_obj, id_compile_evaluate, 1, parse_args->body->obj); + rb_funcall(variable_obj, id_compile_evaluate, 1, parse_args->code_obj); if (code->stack_size != code->protected_stack_size + 1) { rb_raise(rb_eRuntimeError, "Liquid::Variable#compile_evaluate didn't leave exactly 1 new element on the stack"); } @@ -114,7 +114,7 @@ static VALUE variable_strict_parse_rescue(VALUE uncast_args, VALUE exception) void internal_variable_compile_evaluate(variable_parse_args_t *parse_args) { - vm_assembler_t *code = &parse_args->body->code; + vm_assembler_t *code = parse_args->code; variable_strict_parse_rescue_t rescue_args = { .parse_args = parse_args, .instructions_size = c_buffer_size(&code->instructions), @@ -126,7 +126,7 @@ void internal_variable_compile_evaluate(variable_parse_args_t *parse_args) void internal_variable_compile(variable_parse_args_t *parse_args, unsigned int line_number) { - vm_assembler_t *code = &parse_args->body->code; + vm_assembler_t *code = parse_args->code; vm_assembler_add_render_variable_rescue(code, line_number); internal_variable_compile_evaluate(parse_args); vm_assembler_add_pop_write(code); diff --git a/ext/liquid_c/variable.h b/ext/liquid_c/variable.h index df9c3c13..b39e2563 100644 --- a/ext/liquid_c/variable.h +++ b/ext/liquid_c/variable.h @@ -7,7 +7,8 @@ typedef struct variable_parse_args { const char *markup; const char *markup_end; - block_body_t *body; + vm_assembler_t *code; + VALUE code_obj; VALUE parse_context; } variable_parse_args_t; From ef0aea5c29d473af5b074d9d3faebcf05aa42c71 Mon Sep 17 00:00:00 2001 From: Dylan Thacker-Smith Date: Thu, 29 Oct 2020 10:41:22 -0400 Subject: [PATCH 3/3] Strict parse Liquid::Variable.new objects to a Liquid::C::Expression which embeds the filtering instructions --- ext/liquid_c/variable.c | 44 +++++++++++++++++++++++++++++++++++++++++ lib/liquid/c.rb | 13 ++++++++++-- 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/ext/liquid_c/variable.c b/ext/liquid_c/variable.c index 40bb978b..6c93b3e0 100644 --- a/ext/liquid_c/variable.c +++ b/ext/liquid_c/variable.c @@ -6,6 +6,12 @@ static ID id_rescue_strict_parse_syntax_error; +static ID id_ivar_parse_context; +static ID id_ivar_name; +static ID id_ivar_filters; + +static VALUE frozen_empty_array; + static VALUE try_variable_strict_parse(VALUE uncast_args) { variable_parse_args_t *parse_args = (void *)uncast_args; @@ -132,8 +138,46 @@ void internal_variable_compile(variable_parse_args_t *parse_args, unsigned int l vm_assembler_add_pop_write(code); } +static VALUE variable_strict_parse_method(VALUE self, VALUE markup) +{ + StringValue(markup); + check_utf8_encoding(markup, "markup"); + + VALUE parse_context = rb_ivar_get(self, id_ivar_parse_context); + + expression_t *expression; + VALUE expression_obj = expression_new(&expression); + + variable_parse_args_t parse_args = { + .markup = RSTRING_PTR(markup), + .markup_end = RSTRING_END(markup), + .code = &expression->code, + .code_obj = expression_obj, + .parse_context = parse_context, + }; + try_variable_strict_parse((VALUE)&parse_args); + RB_GC_GUARD(markup); + assert(expression->code.stack_size == 1); + vm_assembler_add_leave(&expression->code); + + rb_ivar_set(self, id_ivar_name, expression_obj); + rb_ivar_set(self, id_ivar_filters, frozen_empty_array); + + return Qnil; +} + void init_liquid_variable(void) { id_rescue_strict_parse_syntax_error = rb_intern("rescue_strict_parse_syntax_error"); + + id_ivar_parse_context = rb_intern("@parse_context"); + id_ivar_name = rb_intern("@name"); + id_ivar_filters = rb_intern("@filters"); + + frozen_empty_array = rb_ary_new(); + rb_ary_freeze(frozen_empty_array); + rb_global_variable(&frozen_empty_array); + + rb_define_method(cLiquidVariable, "c_strict_parse", variable_strict_parse_method, 1); } diff --git a/lib/liquid/c.rb b/lib/liquid/c.rb index 38f4e5f0..b23fcb93 100644 --- a/lib/liquid/c.rb +++ b/lib/liquid/c.rb @@ -68,8 +68,7 @@ def new_block_body end end - private - + # @api private def disable_liquid_c_nodes # Liquid::Profiler exposes the internal parse tree that we don't want to build when # parsing with liquid-c, so consider liquid-c to be disabled when using it. @@ -140,6 +139,16 @@ def set_error_mode(parse_context, mode) parse_context.instance_variable_set(:@error_mode, mode) end end + + alias_method :ruby_strict_parse, :strict_parse + + def strict_parse(markup) + if parse_context.disable_liquid_c_nodes + ruby_strict_parse(markup) + else + c_strict_parse(markup) + end + end end Liquid::StrainerTemplate.class_eval do