maria-developers team mailing list archive

[Aleksey Midenkov <midenok@xxxxxxxxx>]

Hi Sergei!

The updated patches are here:

https://github.com/MariaDB/server/commits/2c2e79487a9189e02fce8f884a9ab2b42b9aa333


On Tue, Nov 3, 2020 at 8:28 PM Aleksey Midenkov <midenok@xxxxxxxxx> wrote:
>
> Sergei,
>
> the updated patches are here:
>
>
https://github.com/MariaDB/server/commits/5f6bcd6cd0145513974b0dfc5b2cefba28b72f1a
>
> On Mon, Nov 2, 2020 at 11:33 PM Sergei Golubchik <serg@xxxxxxxxxxx> 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
MDEV-18734.
> >
...
>
> > > 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
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