1berry/MariaDB-server
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Optimize usage of c_ptr(), c_ptr_quick() and String::alloc()

Browse Source 
The problem was that when one used String::alloc() to allocate a string,
the String ensures that there is space for an extra NULL byte in the
buffer and if not, reallocates the string. This is a problem with the
String::set_int() that calls alloc(21), which forces extra
malloc/free calls to happen.

- We do not anymore re-allocate String if alloc() is called with the
  Allocated_length. This reduces number of malloc() allocations,
  especially one big re-allocation in Protocol::send_result_Set_metadata()
  for almost every query that produced a result to the connnected client.
- Avoid extra mallocs when using LONGLONG_BUFFER_SIZE
  This can now be done as alloc() doesn't increase buffers if new length is
  not bigger than old one.
- c_ptr() is redesigned to be safer (but a bit longer) than before.
- Remove wrong usage of c_ptr_quick()
  c_ptr_quick() was used in many cases to get the pointer to the used
  buffer, even when it didn't need to be \0 terminated. In this case
  ptr() is a better substitute.
  Another problem with c_ptr_quick() is that it did not guarantee that
  the string would be \0 terminated.
- item_val_str(), an API function not used currently by the server,
  now always returns a null terminated string (before it didn't always
  do that).
- Ensure that all String allocations uses STRING_PSI_MEMORY_KEY. The old
  mixed usage of performance keys caused assert's when String buffers
  where shrunk.
- Binary_string::shrink() is simplifed
- Fixed bug in String(const char *str, size_t len, CHARSET_INFO *cs) that
  used Binary_string((char *) str, len) instead of Binary_string(str,len).
- Changed argument to String() creations and String.set() functions to use
  'const char*' instead of 'char*'. This ensures that Alloced_length is
  not set, which gives safety against someone trying to change the
  original string. This also would allow us to use !Alloced_length in
  c_ptr() if needed.
- Changed string_ptr_cmp() to use memcmp() instead of c_ptr() to avoid
  a possible malloc during string comparision.
This commit is contained in:
Monty 2020-09-16 11:23:50 +03:00 committed by Sergei Golubchik
parent da85ad7987
commit 36cdd5c3cd
15 changed files with 123 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
sql/field_conv.cc
View File

@ -369,7 +369,7 @@ void Field::do_field_string(Copy_field *copy)
res.length(0U);
copy->from_field->val_str(&res);
copy->to_field->store(res.c_ptr_quick(), res.length(), res.charset());
copy->to_field->store(res.ptr(), res.length(), res.charset());
}
@ -389,10 +389,10 @@ static void do_field_varbinary_pre50(Copy_field *copy)
copy->from_field->val_str(&copy->tmp);
/* Use the same function as in 4.1 to trim trailing spaces */
size_t length= my_lengthsp_8bit(&my_charset_bin, copy->tmp.c_ptr_quick(),
size_t length= my_lengthsp_8bit(&my_charset_bin, copy->tmp.ptr(),
copy->from_field->field_length);
copy->to_field->store(copy->tmp.c_ptr_quick(), length,
copy->to_field->store(copy->tmp.ptr(), length,
copy->tmp.charset());
}

4
sql/item.cc
View File

@ -4599,7 +4599,7 @@ const String *Item_param::value_query_val_str(THD *thd, String *str) const
break;
}
DBUG_ASSERT(str->length() <= typelen);
buf= str->c_ptr_quick();
buf= (char*) str->ptr();
ptr= buf + str->length();
*ptr++= '\'';
ptr+= (uint) my_TIME_to_str(&value.time, ptr, decimals);
@ -4705,7 +4705,7 @@ Item *Item_param::value_clone_item(THD *thd)
return 0; // Should create Item_decimal. See MDEV-11361.
case STRING_RESULT:
return new (mem_root) Item_string(thd, name,
Lex_cstring(value.m_string.c_ptr_quick(),
Lex_cstring(value.m_string.ptr(),
value.m_string.length()),
value.m_string.charset(),
collation.derivation,

11
sql/partition_info.cc
View File

@ -126,7 +126,7 @@ partition_info *partition_info::get_clone(THD *thd)
/**
Mark named [sub]partition to be used/locked.
@param part_name Partition name to match.
@param part_name Partition name to match. Must be \0 terminated!
@param length Partition name length.
@return Success if partition found
@ -140,7 +140,10 @@ bool partition_info::add_named_partition(const char *part_name, size_t length)
PART_NAME_DEF *part_def;
Partition_share *part_share;
DBUG_ENTER("partition_info::add_named_partition");
DBUG_ASSERT(table && table->s && table->s->ha_share);
DBUG_ASSERT(part_name[length] == 0);
DBUG_ASSERT(table);
DBUG_ASSERT(table->s);
DBUG_ASSERT(table->s->ha_share);
part_share= static_cast<Partition_share*>((table->s->ha_share));
DBUG_ASSERT(part_share->partition_name_hash_initialized);
part_name_hash= &part_share->partition_name_hash;
@ -172,9 +175,9 @@ bool partition_info::add_named_partition(const char *part_name, size_t length)
else
bitmap_set_bit(&read_partitions, part_def->part_id);
}
DBUG_PRINT("info", ("Found partition %u is_subpart %d for name %s",
DBUG_PRINT("info", ("Found partition %u is_subpart %d for name %.*s",
part_def->part_id, part_def->is_subpart,
part_name));
length, part_name));
DBUG_RETURN(false);
}

2
sql/sql_analyze_stmt.cc
View File

@ -81,7 +81,7 @@ void Filesort_tracker::print_json_members(Json_writer *writer)
}
get_data_format(&str);
writer->add_member("r_sort_mode").add_str(str.c_ptr(), str.length());
writer->add_member("r_sort_mode").add_str(str.ptr(), str.length());
}
void Filesort_tracker::get_data_format(String *str)

7
sql/sql_help.cc
View File

@ -541,7 +541,12 @@ extern "C" int string_ptr_cmp(const void* ptr1, const void* ptr2)
{
String *str1= *(String**)ptr1;
String *str2= *(String**)ptr2;
return strcmp(str1->c_ptr(),str2->c_ptr());
uint length1= str1->length();
uint length2= str2->length();
int tmp= memcmp(str1->ptr(),str2->ptr(), MY_MIN(length1, length2));
if (tmp)
return tmp;
return (int) length2 - (int) length1;
}
/*

8
sql/sql_plugin.cc
View File

@ -404,12 +404,16 @@ static int item_value_type(struct st_mysql_value *value)
static const char *item_val_str(struct st_mysql_value *value,
char *buffer, int *length)
{
String str(buffer, *length, system_charset_info), *res;
size_t org_length= *length;
String str(buffer, org_length, system_charset_info), *res;
if (!(res= ((st_item_value_holder*)value)->item->val_str(&str)))
return NULL;
*length= res->length();
if (res->c_ptr_quick() == buffer)
if (res->ptr() == buffer && res->length() < org_length)
{
buffer[res->length()]= 0;
return buffer;
}
/*
Lets be nice and create a temporary string since the

6
sql/sql_repl.cc
View File

@ -2153,7 +2153,7 @@ static int init_binlog_sender(binlog_send_info *info,
"Start binlog_dump to slave_server(%lu), pos(%s, %lu), "
"using_gtid(%d), gtid('%s')", thd->variables.server_id,
log_ident, (ulong)*pos, info->using_gtid_state,
connect_gtid_state.c_ptr_quick());
connect_gtid_state.c_ptr_safe());
}
#ifndef DBUG_OFF
@ -2176,7 +2176,7 @@ static int init_binlog_sender(binlog_send_info *info,
const char *name=search_file_name;
if (info->using_gtid_state)
{
if (info->gtid_state.load(connect_gtid_state.c_ptr_quick(),
if (info->gtid_state.load(connect_gtid_state.ptr(),
connect_gtid_state.length()))
{
info->errmsg= "Out of memory or malformed slave request when obtaining "
@ -2185,7 +2185,7 @@ static int init_binlog_sender(binlog_send_info *info,
return 1;
}
if (info->until_gtid_state &&
info->until_gtid_state->load(slave_until_gtid_str.c_ptr_quick(),
info->until_gtid_state->load(slave_until_gtid_str.ptr(),
slave_until_gtid_str.length()))
{
info->errmsg= "Out of memory or malformed slave request when "

2
sql/sql_select.cc
View File

@ -27860,7 +27860,7 @@ void TABLE_LIST::print(THD *thd, table_map eliminated_tables, String *str,
for (i= 1; i <= num_parts; i++)
{
String *name= name_it++;
append_identifier(thd, str, name->c_ptr(), name->length());
append_identifier(thd, str, name->ptr(), name->length());
if (i != num_parts)
str->append(',');
}

2
sql/sql_show.cc
View File

@ -10121,7 +10121,7 @@ char *thd_get_error_context_description(THD *thd, char *buffer,
*/
DBUG_ASSERT(buffer != NULL);
length= MY_MIN(str.length(), length-1);
memcpy(buffer, str.c_ptr_quick(), length);
memcpy(buffer, str.ptr(), length);
/* Make sure that the new string is null terminated */
buffer[length]= '\0';
return buffer;

54
sql/sql_string.cc
View File

@ -41,7 +41,7 @@ bool Binary_string::real_alloc(size_t length)
if (Alloced_length < arg_length)
{
free();
if (!(Ptr=(char*) my_malloc(PSI_INSTRUMENT_ME,
if (!(Ptr=(char*) my_malloc(STRING_PSI_MEMORY_KEY,
arg_length,MYF(MY_WME | (thread_specific ?
MY_THREAD_SPECIFIC : 0)))))
return TRUE;
@ -55,7 +55,8 @@ bool Binary_string::real_alloc(size_t length)
/**
Allocates a new buffer on the heap for this String.
Allocates a new buffer on the heap for this String if current buffer is
smaller.
- If the String's internal buffer is privately owned and heap allocated,
one of the following is performed.
@ -70,7 +71,8 @@ bool Binary_string::real_alloc(size_t length)
will be allocated and the string copied accoring to its length, as found
in String::length().
For C compatibility, the new string buffer is null terminated.
For C compatibility, the new string buffer is null terminated if it was
allocated.
@param alloc_length The requested string size in characters, excluding any
null terminator.
@ -81,9 +83,10 @@ bool Binary_string::real_alloc(size_t length)
@retval true An error occurred when attempting to allocate memory.
*/
bool Binary_string::realloc_raw(size_t alloc_length)
{
if (Alloced_length <= alloc_length)
if (Alloced_length < alloc_length)
{
char *new_ptr;
uint32 len= ALIGN_SIZE(alloc_length+1);
@ -92,13 +95,13 @@ bool Binary_string::realloc_raw(size_t alloc_length)
return TRUE; /* Overflow */
if (alloced)
{
if (!(new_ptr= (char*) my_realloc(PSI_INSTRUMENT_ME, Ptr,len,
if (!(new_ptr= (char*) my_realloc(STRING_PSI_MEMORY_KEY, Ptr,len,
MYF(MY_WME |
(thread_specific ?
MY_THREAD_SPECIFIC : 0)))))
return TRUE; // Signal error
}
else if ((new_ptr= (char*) my_malloc(PSI_INSTRUMENT_ME, len,
else if ((new_ptr= (char*) my_malloc(STRING_PSI_MEMORY_KEY, len,
MYF(MY_WME |
(thread_specific ?
MY_THREAD_SPECIFIC : 0)))))
@ -118,9 +121,14 @@ bool Binary_string::realloc_raw(size_t alloc_length)
return FALSE;
}
bool String::set_int(longlong num, bool unsigned_flag, CHARSET_INFO *cs)
{
uint l=20*cs->mbmaxlen+1;
/*
This allocates a few bytes extra in the unlikely case that cs->mb_maxlen
> 1, but we can live with that
*/
uint l= LONGLONG_BUFFER_SIZE * cs->mbmaxlen;
int base= unsigned_flag ? 10 : -10;
if (alloc(l))
@ -235,7 +243,7 @@ bool Binary_string::copy()
*/
bool Binary_string::copy(const Binary_string &str)
{
if (alloc(str.str_length))
if (alloc(str.str_length+1))
return TRUE;
if ((str_length=str.str_length))
bmove(Ptr,str.Ptr,str_length); // May be overlapping
@ -246,7 +254,7 @@ bool Binary_string::copy(const Binary_string &str)
bool Binary_string::copy(const char *str, size_t arg_length)
{
DBUG_ASSERT(arg_length < UINT_MAX32);
if (alloc(arg_length))
if (alloc(arg_length+1))
return TRUE;
if (Ptr == str && arg_length == uint32(str_length))
{
@ -272,7 +280,7 @@ bool Binary_string::copy(const char *str, size_t arg_length)
bool Binary_string::copy_or_move(const char *str, size_t arg_length)
{
DBUG_ASSERT(arg_length < UINT_MAX32);
if (alloc(arg_length))
if (alloc(arg_length+1))
return TRUE;
if ((str_length=uint32(arg_length)))
memmove(Ptr,str,arg_length);
@ -1251,24 +1259,16 @@ bool String::append_semi_hex(const char *s, uint len, CHARSET_INFO *cs)
return false;
}
// Shrink the buffer, but only if it is allocated on the heap.
void Binary_string::shrink(size_t arg_length)
{
if (!is_alloced())
return;
if (ALIGN_SIZE(arg_length + 1) < Alloced_length)
{
char* new_ptr;
if (!(new_ptr = (char*)my_realloc(STRING_PSI_MEMORY_KEY, Ptr, arg_length,
MYF(thread_specific ? MY_THREAD_SPECIFIC : 0))))
{
Alloced_length = 0;
real_alloc(arg_length);
}
else
{
Ptr = new_ptr;
Alloced_length = (uint32)arg_length;
}
}
if (is_alloced() && ALIGN_SIZE(arg_length + 1) < Alloced_length)
{
/* my_realloc() can't fail as new buffer is less than the original one */
Ptr= (char*) my_realloc(STRING_PSI_MEMORY_KEY, Ptr, arg_length,
MYF(thread_specific ?
MY_THREAD_SPECIFIC : 0));
Alloced_length= (uint32) arg_length;
}
}

61
sql/sql_string.h
View File

@ -600,25 +600,55 @@ public:
inline char *c_ptr()
{
DBUG_ASSERT(!alloced || !Ptr || !Alloced_length ||
(Alloced_length >= (str_length + 1)));
if (unlikely(!Ptr))
return (char*) "";
/*
Here we assume that any buffer used to initalize String has
an end \0 or have at least an accessable character at end.
This is to handle the case of String("Hello",5) and
String("hello",5) efficiently.
if (!Ptr || Ptr[str_length]) // Should be safe
(void) realloc(str_length);
We have two options here. To test for !Alloced_length or !alloced.
Using "Alloced_length" is slightly safer so that we do not read
from potentially unintialized memory (normally not dangerous but
may give warnings in valgrind), but "alloced" is safer as there
are less change to get memory loss from code that is using
String((char*), length) or String.set((char*), length) and does
not free things properly (and there is several places in the code
where this happens and it is hard to find out if any of these will call
c_ptr().
*/
if (unlikely(!alloced && !Ptr[str_length]))
return Ptr;
if (str_length < Alloced_length)
{
Ptr[str_length]=0;
return Ptr;
}
(void) realloc(str_length+1); /* This will add end \0 */
return Ptr;
}
/*
One should use c_ptr() instead for most cases. This will be deleted soon,
kept for compatiblity.
*/
inline char *c_ptr_quick()
{
if (Ptr && str_length < Alloced_length)
Ptr[str_length]=0;
return Ptr;
return c_ptr_safe();
}
/*
This is to be used only in the case when one cannot use c_ptr().
The cases are:
- When one initializes String with an external buffer and length and
buffer[length] could be uninitalized when c_ptr() is called.
- When valgrind gives warnings about uninitialized memory with c_ptr().
*/
inline char *c_ptr_safe()
{
if (Ptr && str_length < Alloced_length)
Ptr[str_length]=0;
else
(void) realloc(str_length);
(void) realloc(str_length + 1);
return Ptr;
}
@ -634,7 +664,16 @@ public:
}
inline bool alloc(size_t arg_length)
{
if (arg_length < Alloced_length)
/*
Allocate if we need more space or if we don't have p_done any
allocation yet (we don't want to have Ptr to be NULL for empty strings).
Note that if arg_length == Alloced_length then we don't allocate.
This ensures we don't do any extra allocations in protocol and String:int,
but the string will not be atomically null terminated if c_ptr() is not
called.
*/
if (arg_length <= Alloced_length && Alloced_length)
return 0;
return real_alloc(arg_length);
}
@ -642,7 +681,7 @@ public:
bool realloc_raw(size_t arg_length);
bool realloc(size_t arg_length)
{
if (realloc_raw(arg_length))
if (realloc_raw(arg_length+1))
return TRUE;
Ptr[arg_length]= 0; // This make other funcs shorter
return FALSE;
@ -743,7 +782,7 @@ public:
*/
String(const char *str, size_t len, CHARSET_INFO *cs)
:Charset(cs),
Binary_string((char *) str, len)
Binary_string(str, len)
{ }
String(char *str, size_t len, CHARSET_INFO *cs)
:Charset(cs),

2
sql/sql_type.cc
View File

@ -9258,7 +9258,7 @@ Type_handler_general_purpose_int::partition_field_append_value(
const
{
DBUG_ASSERT(item_expr->cmp_type() == INT_RESULT);
StringBuffer<21> tmp;
StringBuffer<LONGLONG_BUFFER_SIZE> tmp;
longlong value= item_expr->val_int();
tmp.set(value, system_charset_info);
return str->append(tmp);

23
sql/sql_yacc.yy
View File

@ -12077,18 +12077,18 @@ using_list:
{
if (unlikely(!($$= new (thd->mem_root) List<String>)))
MYSQL_YYABORT;
String *s= new (thd->mem_root) String((const char *) $1.str,
$1.length,
system_charset_info);
String *s= new (thd->mem_root) String((const char*) $1.str,
$1.length,
system_charset_info);
if (unlikely(unlikely(s == NULL)))
MYSQL_YYABORT;
$$->push_back(s, thd->mem_root);
}
| using_list ',' ident
{
String *s= new (thd->mem_root) String((const char *) $3.str,
$3.length,
system_charset_info);
String *s= new (thd->mem_root) String((const char*) $3.str,
$3.length,
system_charset_info);
if (unlikely(unlikely(s == NULL)))
MYSQL_YYABORT;
if (unlikely($1->push_back(s, thd->mem_root)))
@ -14256,8 +14256,9 @@ wild_and_where:
/* empty */ { $$= 0; }
| LIKE remember_tok_start TEXT_STRING_sys
{
Lex->wild= new (thd->mem_root) String($3.str, $3.length,
system_charset_info);
Lex->wild= new (thd->mem_root) String((const char*) $3.str,
$3.length,
system_charset_info);
if (unlikely(Lex->wild == NULL))
MYSQL_YYABORT;
$$= $2;
@ -14931,9 +14932,9 @@ text_literal:
text_string:
TEXT_STRING_literal
{
$$= new (thd->mem_root) String($1.str,
$1.length,
thd->variables.collation_connection);
$$= new (thd->mem_root) String((const char*) $1.str,
$1.length,
thd->variables.collation_connection);
if (unlikely($$ == NULL))
MYSQL_YYABORT;
}

2
sql/table.cc
View File

@ -4841,7 +4841,7 @@ rename_file_ext(const char * from,const char * to,const char * ext)
bool get_field(MEM_ROOT *mem, Field *field, String *res)
{
char *to;
const char *to;
StringBuffer<MAX_FIELD_WIDTH> str;
bool rc;
THD *thd= field->get_thd();

4
sql/wsrep_schema.cc
View File

@ -1250,7 +1250,7 @@ int Wsrep_schema::replay_transaction(THD* orig_thd,
{
Wsrep_schema_impl::thd_context_switch thd_context_switch(&thd, orig_thd);
ret= wsrep_apply_events(orig_thd, rli, buf.c_ptr_quick(), buf.length());
ret= wsrep_apply_events(orig_thd, rli, buf.ptr(), buf.length());
if (ret)
{
WSREP_WARN("Wsrep_schema::replay_transaction: failed to apply fragments");
@ -1404,7 +1404,7 @@ int Wsrep_schema::recover_sr_transactions(THD *orig_thd)
String data_str;
(void)frag_table->field[4]->val_str(&data_str);
wsrep::const_buffer data(data_str.c_ptr_quick(), data_str.length());
wsrep::const_buffer data(data_str.ptr(), data_str.length());
wsrep::ws_meta ws_meta(gtid,
wsrep::stid(server_id,
transaction_id,