Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimization for layout_init() #6547

Merged
merged 6 commits into from Aug 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions ruby/ext/google/protobuf_c/defs.c
Expand Up @@ -487,6 +487,9 @@ void Descriptor_mark(void* _self) {
Descriptor* self = _self;
rb_gc_mark(self->klass);
rb_gc_mark(self->descriptor_pool);
if (self->layout && self->layout->empty_template) {
layout_mark(self->layout, self->layout->empty_template);
}
}

void Descriptor_free(void* _self) {
Expand Down
2 changes: 1 addition & 1 deletion ruby/ext/google/protobuf_c/encode_decode.c
Expand Up @@ -693,7 +693,7 @@ void add_handlers_for_message(const void *closure, upb_handlers *h) {
// class is actually built, so to work around this, we just create the layout
// (and handlers, in the class-building function) on-demand.
if (desc->layout == NULL) {
desc->layout = create_layout(desc);
create_layout(desc);
}

// If this is a mapentry message type, set up a special set of handlers and
Expand Down
2 changes: 1 addition & 1 deletion ruby/ext/google/protobuf_c/map.c
Expand Up @@ -495,7 +495,7 @@ VALUE Map_length(VALUE _self) {
return ULL2NUM(upb_strtable_count(&self->table));
}

static VALUE Map_new_this_type(VALUE _self) {
VALUE Map_new_this_type(VALUE _self) {
Map* self = ruby_to_Map(_self);
VALUE new_map = Qnil;
VALUE key_type = fieldtype_to_ruby(self->key_type);
Expand Down
22 changes: 10 additions & 12 deletions ruby/ext/google/protobuf_c/message.c
Expand Up @@ -62,25 +62,19 @@ VALUE Message_alloc(VALUE klass) {
Descriptor* desc = ruby_to_Descriptor(descriptor);
MessageHeader* msg;
VALUE ret;
size_t size;

if (desc->layout == NULL) {
desc->layout = create_layout(desc);
create_layout(desc);
}

msg = (MessageHeader*)ALLOC_N(uint8_t,
sizeof(MessageHeader) + desc->layout->size);

memset(Message_data(msg), 0, desc->layout->size);

// We wrap first so that everything in the message object is GC-rooted in case
// a collection happens during object creation in layout_init().
ret = TypedData_Wrap_Struct(klass, &Message_type, msg);
msg = ALLOC_N(uint8_t, sizeof(MessageHeader) + desc->layout->size);
msg->descriptor = desc;
rb_ivar_set(ret, descriptor_instancevar_interned, descriptor);

msg->unknown_fields = NULL;
memcpy(Message_data(msg), desc->layout->empty_template, desc->layout->size);

layout_init(desc->layout, Message_data(msg));
ret = TypedData_Wrap_Struct(klass, &Message_type, msg);
rb_ivar_set(ret, descriptor_instancevar_interned, descriptor);

return ret;
}
Expand Down Expand Up @@ -472,7 +466,11 @@ int Message_initialize_kwarg(VALUE key, VALUE val, VALUE _self) {
* Message class are provided on each concrete message class.
*/
VALUE Message_initialize(int argc, VALUE* argv, VALUE _self) {
MessageHeader* self;
VALUE hash_args;
TypedData_Get_Struct(_self, MessageHeader, &Message_type, self);

layout_init(self->descriptor->layout, Message_data(self));

if (argc == 0) {
return Qnil;
Expand Down
7 changes: 6 additions & 1 deletion ruby/ext/google/protobuf_c/protobuf.h
Expand Up @@ -419,6 +419,7 @@ extern VALUE cRepeatedField;

RepeatedField* ruby_to_RepeatedField(VALUE value);

VALUE RepeatedField_new_this_type(VALUE _self);
VALUE RepeatedField_each(VALUE _self);
VALUE RepeatedField_index(int argc, VALUE* argv, VALUE _self);
void* RepeatedField_index_native(VALUE _self, int index);
Expand Down Expand Up @@ -467,6 +468,7 @@ extern VALUE cMap;

Map* ruby_to_Map(VALUE value);

VALUE Map_new_this_type(VALUE _self);
VALUE Map_each(VALUE _self);
VALUE Map_keys(VALUE _self);
VALUE Map_values(VALUE _self);
Expand Down Expand Up @@ -516,16 +518,19 @@ struct MessageOneof {
struct MessageLayout {
const Descriptor* desc;
const upb_msgdef* msgdef;
void* empty_template; // Can memcpy() onto a layout to clear it.
MessageField* fields;
MessageOneof* oneofs;
uint32_t size;
uint32_t value_offset;
int value_count;
int repeated_count;
int map_count;
};

#define ONEOF_CASE_MASK 0x80000000

MessageLayout* create_layout(const Descriptor* desc);
void create_layout(Descriptor* desc);
void free_layout(MessageLayout* layout);
bool field_contains_hasbit(MessageLayout* layout,
const upb_fielddef* field);
Expand Down
2 changes: 1 addition & 1 deletion ruby/ext/google/protobuf_c/repeated_field.c
Expand Up @@ -323,7 +323,7 @@ VALUE RepeatedField_length(VALUE _self) {
return INT2NUM(self->size);
}

static VALUE RepeatedField_new_this_type(VALUE _self) {
VALUE RepeatedField_new_this_type(VALUE _self) {
RepeatedField* self = ruby_to_RepeatedField(_self);
VALUE new_rptfield = Qnil;
VALUE element_type = fieldtype_to_ruby(self->field_type);
Expand Down
71 changes: 60 additions & 11 deletions ruby/ext/google/protobuf_c/storage.c
Expand Up @@ -478,7 +478,7 @@ bool is_value_field(const upb_fielddef* f) {
upb_fielddef_isstring(f);
}

MessageLayout* create_layout(const Descriptor* desc) {
void create_layout(Descriptor* desc) {
const upb_msgdef *msgdef = desc->msgdef;
MessageLayout* layout = ALLOC(MessageLayout);
int nfields = upb_msgdef_numfields(msgdef);
Expand All @@ -488,7 +488,10 @@ MessageLayout* create_layout(const Descriptor* desc) {
size_t off = 0;
size_t hasbit = 0;

layout->empty_template = NULL;
layout->desc = desc;
desc->layout = layout;

layout->fields = ALLOC_N(MessageField, nfields);
layout->oneofs = NULL;

Expand All @@ -514,14 +517,49 @@ MessageLayout* create_layout(const Descriptor* desc) {

off = align_up_to(off, sizeof(VALUE));
layout->value_offset = off;
layout->repeated_count = 0;
layout->map_count = 0;
layout->value_count = 0;

// Place all (non-oneof) VALUE fields first.
// Place all VALUE fields for repeated fields.
for (upb_msg_field_begin(&it, msgdef);
!upb_msg_field_done(&it);
upb_msg_field_next(&it)) {
const upb_fielddef* field = upb_msg_iter_field(&it);
if (upb_fielddef_containingoneof(field) || !upb_fielddef_isseq(field) ||
upb_fielddef_ismap(field)) {
continue;
}

layout->fields[upb_fielddef_index(field)].offset = off;
off += sizeof(VALUE);
layout->repeated_count++;
}

// Place all VALUE fields for map fields.
for (upb_msg_field_begin(&it, msgdef);
!upb_msg_field_done(&it);
upb_msg_field_next(&it)) {
const upb_fielddef* field = upb_msg_iter_field(&it);
if (upb_fielddef_containingoneof(field) || !upb_fielddef_isseq(field) ||
!upb_fielddef_ismap(field)) {
continue;
}

layout->fields[upb_fielddef_index(field)].offset = off;
off += sizeof(VALUE);
layout->map_count++;
}

layout->value_count = layout->repeated_count + layout->map_count;

// Next place all other (non-oneof) VALUE fields.
for (upb_msg_field_begin(&it, msgdef);
!upb_msg_field_done(&it);
upb_msg_field_next(&it)) {
const upb_fielddef* field = upb_msg_iter_field(&it);
if (upb_fielddef_containingoneof(field) || !is_value_field(field)) {
if (upb_fielddef_containingoneof(field) || !is_value_field(field) ||
upb_fielddef_isseq(field)) {
continue;
}

Expand Down Expand Up @@ -600,10 +638,19 @@ MessageLayout* create_layout(const Descriptor* desc) {
layout->size = off;
layout->msgdef = msgdef;

return layout;
// Create the empty message template.
layout->empty_template = ALLOC_N(char, layout->size);
memset(layout->empty_template, 0, layout->size);

for (upb_msg_field_begin(&it, layout->msgdef);
!upb_msg_field_done(&it);
upb_msg_field_next(&it)) {
layout_clear(layout, layout->empty_template, upb_msg_iter_field(&it));
}
}

void free_layout(MessageLayout* layout) {
xfree(layout->empty_template);
xfree(layout->fields);
xfree(layout->oneofs);
xfree(layout);
Expand Down Expand Up @@ -896,14 +943,16 @@ void layout_set(MessageLayout* layout,
}
}

void layout_init(MessageLayout* layout,
void* storage) {
void layout_init(MessageLayout* layout, void* storage) {
VALUE* value = (VALUE*)CHARPTR_AT(storage, layout->value_offset);
int i;

upb_msg_field_iter it;
for (upb_msg_field_begin(&it, layout->msgdef);
!upb_msg_field_done(&it);
upb_msg_field_next(&it)) {
layout_clear(layout, storage, upb_msg_iter_field(&it));
for (i = 0; i < layout->repeated_count; i++, value++) {
*value = RepeatedField_new_this_type(*value);
}

for (i = 0; i < layout->map_count; i++, value++) {
*value = Map_new_this_type(*value);
}
}

Expand Down