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

fix #292: bad alloc performance due to excessive memset #293

Merged
merged 3 commits into from
Apr 6, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion ionc/ion_allocation.c
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ void *_ion_alloc_with_owner_helper(ION_ALLOCATION_CHAIN *powner, SIZE request_le
ptr = pblock->position;
pblock->position = next_ptr;

if (ptr) memset(ptr, 0, length);
return ptr;
}

Expand Down
5 changes: 2 additions & 3 deletions ionc/ion_collection.c
Original file line number Diff line number Diff line change
Expand Up @@ -308,10 +308,9 @@ ION_COLLECTION_NODE *_ion_collection_alloc_node_helper( ION_COLLECTION *collecti
}

// clean up the node before anyone uses it
node->_next = NULL;
node->_prev = NULL;
memset(node, 0, collection->_node_size);

return node;
return node;
}

ION_COLLECTION_NODE *_ion_collection_push_node_helper(ION_COLLECTION *collection)
Expand Down
5 changes: 5 additions & 0 deletions ionc/ion_reader_text.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ iERR _ion_reader_text_open(ION_READER *preader)
, &(text->_field_name_buffer_length)
));
text->_field_name.sid = UNKNOWN_SID;
ION_STRING_INIT(&text->_field_name.import_location.name);
text->_field_name.import_location.location = UNKNOWN_SID;

text->_annotation_string_pool_length = preader->options.max_annotation_count; // max number of annotations, size of string pool as count
text->_annotation_value_buffer_length = preader->options.max_annotation_buffered + (preader->options.max_annotation_count * sizeof(BYTE));
Expand Down Expand Up @@ -167,6 +169,8 @@ iERR _ion_reader_text_reset_value(ION_READER *preader)
ION_STRING_INIT(&text->_field_name.value);
text->_field_name.add_count = 0;
text->_field_name.sid = UNKNOWN_SID;
ION_STRING_INIT(&text->_field_name.import_location.name);
text->_field_name.import_location.location = UNKNOWN_SID;

text->_value_type = tid_none;
text->_value_sub_type = IST_NONE;
Expand Down Expand Up @@ -1612,6 +1616,7 @@ iERR _ion_reader_text_read_symbol(ION_READER *preader, ION_SYMBOL *p_symbol)
ION_STRING_ASSIGN(&p_symbol->value, user_str);
p_symbol->sid = UNKNOWN_SID;
ION_STRING_INIT(&p_symbol->import_location.name);
p_symbol->import_location.location = UNKNOWN_SID;
}
iRETURN;
}
Expand Down
4 changes: 4 additions & 0 deletions ionc/ion_symbol_table.c
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,8 @@ iERR _ion_symbol_table_local_load_symbol_list(ION_READER *preader, hOWNER owner,
ION_STRING_ASSIGN(&sym->value, &str);
}
sym->sid = UNKNOWN_SID;
ION_STRING_INIT(&sym->import_location.name);
sym->import_location.location = UNKNOWN_SID;
}
// step back out to the symbol table struct
IONCHECK(_ion_reader_step_out_helper(preader));
Expand Down Expand Up @@ -510,6 +512,8 @@ iERR _ion_symbol_table_append(ION_READER *preader, hOWNER owner, ION_SYMBOL_TABL
if (!symbol_to_append) break;
appended_symbol = (ION_SYMBOL *)_ion_collection_append(&cloned->symbols);
// These strings have the same owner; they can be assigned rather than copied.
ION_STRING_INIT(&appended_symbol->import_location.name);
appended_symbol->import_location.location = UNKNOWN_SID;
ION_STRING_ASSIGN(&appended_symbol->value, &symbol_to_append->value);
appended_symbol->sid = UNKNOWN_SID; // This is assigned correctly later.
}
Expand Down
8 changes: 8 additions & 0 deletions ionc/ion_writer.c
Original file line number Diff line number Diff line change
Expand Up @@ -1125,9 +1125,12 @@ iERR _ion_writer_add_annotation_helper(ION_WRITER *pwriter, ION_STRING *annotati
annotation_symbol = &pwriter->annotations[pwriter->annotation_curr];
ASSERT(annotation_symbol);

ION_STRING_INIT(&annotation_symbol->value);
IONCHECK(ion_strdup(pwriter->_temp_entity_pool, &annotation_symbol->value, annotation));
annotation_symbol->sid = UNKNOWN_SID; // The text is known; the SID is irrelevant.
annotation_symbol->add_count = 0;
ION_STRING_INIT(&annotation_symbol->import_location.name);
annotation_symbol->import_location.location = UNKNOWN_SID;

pwriter->annotation_curr++;

Expand Down Expand Up @@ -1155,6 +1158,8 @@ iERR _ion_writer_add_annotation_sid_helper(ION_WRITER *pwriter, SID sid)
ION_STRING_INIT(&annotation_symbol->value);
annotation_symbol->sid = sid;
annotation_symbol->add_count = 0;
ION_STRING_INIT(&annotation_symbol->import_location.name);
annotation_symbol->import_location.location = UNKNOWN_SID;

pwriter->annotation_curr++;

Expand Down Expand Up @@ -2230,6 +2235,7 @@ iERR _ion_writer_transition_to_symtab_intercept_state(ION_WRITER *pwriter, ION_T
new_import = _ion_collection_append(import_list);
new_import->descriptor.version = 1;
new_import->descriptor.max_id = ION_SYS_SYMBOL_MAX_ID_UNDEFINED;
new_import->shared_symbol_table = NULL;
}
break;
case tid_LIST_INT:
Expand Down Expand Up @@ -3180,6 +3186,8 @@ iERR ion_temp_buffer_alloc(ION_TEMP_BUFFER *temp_buffer, SIZE needed, void **p_p

if (!temp_buffer) FAILWITH(IERR_INVALID_ARG);
if (!p_ptr || needed < 0) FAILWITH(IERR_INVALID_ARG);
// align to pointer size
needed = (SIZE) ((((size_t) needed) + (sizeof(void *) - 1)) & ~(sizeof(void *) - 1));
if (temp_buffer->position + needed >= temp_buffer->limit) FAILWITH(IERR_NO_MEMORY);

buf = temp_buffer->position;
Expand Down
4 changes: 4 additions & 0 deletions ionc/ion_writer_text.c
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,12 @@ iERR _ion_writer_text_initialize_stack(ION_WRITER *pwriter)
,DEFAULT_WRITER_STACK_DEPTH * sizeof(*(TEXTWRITER(pwriter)->_stack_parent_type))
,(void **)&TEXTWRITER(pwriter)->_stack_parent_type)
);
memset(TEXTWRITER(pwriter)->_stack_parent_type, 0, DEFAULT_WRITER_STACK_DEPTH * sizeof(*(TEXTWRITER(pwriter)->_stack_parent_type)));
IONCHECK(ion_temp_buffer_alloc(&pwriter->temp_buffer
,DEFAULT_WRITER_STACK_DEPTH * sizeof(*(TEXTWRITER(pwriter)->_stack_flags))
,(void **)&TEXTWRITER(pwriter)->_stack_flags)
);
memset(TEXTWRITER(pwriter)->_stack_flags, 0, DEFAULT_WRITER_STACK_DEPTH * sizeof(*(TEXTWRITER(pwriter)->_stack_flags)));
iRETURN;
}

Expand All @@ -79,7 +81,9 @@ iERR _ion_writer_text_grow_stack(ION_WRITER *pwriter)
IONCHECK(ion_temp_buffer_alloc(&pwriter->temp_buffer, new_flag_size, (void **)&pnew_flags));

memcpy(pnew_types, TEXTWRITER(pwriter)->_stack_parent_type, old_type_size);
memset(((char *) pnew_types) + old_type_size, 0, new_type_size - old_type_size);
memcpy(pnew_flags, TEXTWRITER(pwriter)->_stack_flags, old_flag_size);
memset(((char *) pnew_flags) + old_flag_size, 0, new_flag_size - old_flag_size);

TEXTWRITER(pwriter)->_stack_parent_type = pnew_types;
TEXTWRITER(pwriter)->_stack_flags = pnew_flags;
Expand Down
1 change: 1 addition & 0 deletions tools/events/ion_event_stream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -850,6 +850,7 @@ iERR ion_event_stream_read_event(hREADER reader, ION_EVENT_READ_PARAMS) {
void *consensus_value = NULL;
uint8_t visited_fields = 0;

ION_SYMBOL_INIT(&value_annotation);
ION_SYMBOL_INIT(&value_field_name);
ION_STRING_INIT(&value_text_str);

Expand Down