Hi Sergei! The updated patches are here: https://github.com/MariaDB/server/commits/2c2e79487a9189e02fce8f884a9ab2b42b... On Tue, Nov 3, 2020 at 8:28 PM Aleksey Midenkov <midenok@gmail.com> wrote:
Sergei,
the updated patches are here:
https://github.com/MariaDB/server/commits/5f6bcd6cd0145513974b0dfc5b2cefba28...
On Mon, Nov 2, 2020 at 11:33 PM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Aleksey!
Don't be confused, the commit header and the comment come from your last commit, but the diff below includes all three commits that mention
...
diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc index 9d82df0235c7..7c658082d397 100644 --- a/sql/ha_partition.cc +++ b/sql/ha_partition.cc @@ -5125,14 +5128,14 @@ bool ha_partition::init_record_priority_queue() uint alloc_len; uint used_parts= bitmap_bits_set(&m_part_info->read_partitions); /* Allocate record buffer for each used partition. */ - m_priority_queue_rec_len= m_rec_length + PARTITION_BYTES_IN_POS; + m_priority_queue_rec_len= m_rec_length + ORDERED_REC_OFFSET; if (!m_using_extended_keys) m_priority_queue_rec_len += m_file[0]->ref_length; alloc_len= used_parts * m_priority_queue_rec_len; /* Allocate a key for temporary use when setting up the scan. */ alloc_len+= table_share->max_key_length;
- if (!(m_ordered_rec_buffer= (uchar*)my_malloc(alloc_len, MYF(MY_WME)))) + if (!(m_ordered_rec_buffer= (uchar*) alloc_root(&m_ordered_root, alloc_len)))
I don't see why you need a memroot here. memroot is needed when you allocate later at difeerent points in time some initially unpredictable amount of memory that has the same lifetime and needs to be freed all at once.
I know... This is just another convenient way of allocation with
MDEV-18734. presumably minimized number of mallocs (probably pre_alloc_size should be tweaked to a better value).
But here it seems that you only allocate once. You could just make
m_priority_queue_rec_len = m_rec_length + PARTITION_BYTES_IN_POS + table->s->blob_fields * sizeof(Ordered_blob_storage)
I don't want this data serialized. It is inconvenient to read the dump of
the record. Why we should make it harder to code, read the code and debug while we can make it easier? If it is only for guaranteed 1 malloc, I don't think it's worth it.
Added another patch that does 1 malloc in m_ordered_root. -- All the best, Aleksey Midenkov @midenok