Re: [PATCH v5 07/10] perf record: implement -z,--compression_level=n option and compression

Jiri Olsa
2019-03-11T10:56:05Z

On Sun, Mar 10, 2019 at 07:17:08PM +0300, Alexey Budankov wrote:

SNIP

> > so to be on the same page.. normal processing without compression is:
> >
> > perf_mmap__push does:
> > push(mmap buf)
> > record__pushfn
> > record__write
> > write(buf)
> >
> > perf_mmap__aio_push does:
> > memcpy(aio buf, mmap buf)
> > push(aio buf)
> > record__aio_pushfn
> > record__aio_write
> > aio_write(aio buf)
> >
> >
> > and for compression it would be:
> >
> > perf_mmap__push does:
> > push(mmap buf)
> > compress_push
> > memcpy(compress buffer, mmapbuf) EXTRA copy
> > record__pushfn
> > record__write
> > write(buf)
> >
> > perf_mmap__aio_push does:
> > memcpy(aio buf, mmap buf)
> > memcpy(compress buffer, mmapbuf) EXTRA copy
> > push(aio buf)
> > record__aio_pushfn
> > record__aio_write
> > aio_write(aio buf)
> >
> >
> > side note: that actualy makes me think why do we even have perf_mmap__aio_push,
> > it looks like we could copy the buf in the callback push function with no harm?
>
> Well, yes, perf_mmap__aio_push() can be avoided and perf_mmap__push() can be used
> as for serial as for AIO, moving all the specifics to record code from mmap.c,
> like this:
>
> Serial
> perf_mmap__push(, record__pushfn)
> push(), possibly two times
> record__pushfn()
> if (-z) zstd_compress(map->base => map->data) <-- compressing memcpy()
> record__write(-z ? map->data, map->base)
> AIO
> record__aio_push()
> perf_mmap__push(, record__aio_pushfn())
> push(), possibly two times
> record__aio_pushfn()
> if (-z) zstd_compress(map->base => map->aio.data[i]) <--- compressing memcpy()
> else memcpy(map->base => map->aio.data[i]) <--- plain memcpy()
> record__aio_write(map->aio.data[i])
>
> So now it looks optimal as from performance and data loss reduction
> perspective as from design perspective. What do you think?

yes, that looks much better.. so we'd have record__pushfn
for standard (serial) and record__aio_pushfn for AIO

thanks,
jirka