Message ID | 20231012062359.1616786-13-irogers@google.com (mailing list archive) |
---|---|
State | Not Applicable |
Headers | show |
Series | Improvements to memory use | expand |
Context | Check | Description |
---|---|---|
netdev/tree_selection | success | Not a local patch |
On Wed, Oct 11, 2023 at 11:24 PM Ian Rogers <irogers@google.com> wrote: > > Zstd streams create dictionaries that can require significant RAM, > especially when there is one per-CPU. Tools like perf record won't use > the streams without the -z option, and so the creation of the streams > is pure overhead. Switch to creating the streams on first use. > > Signed-off-by: Ian Rogers <irogers@google.com> > --- > tools/perf/util/compress.h | 1 + > tools/perf/util/mmap.c | 5 ++-- > tools/perf/util/mmap.h | 1 - > tools/perf/util/zstd.c | 61 ++++++++++++++++++++------------------ > 4 files changed, 35 insertions(+), 33 deletions(-) > > diff --git a/tools/perf/util/compress.h b/tools/perf/util/compress.h > index 0cd3369af2a4..9391850f1a7e 100644 > --- a/tools/perf/util/compress.h > +++ b/tools/perf/util/compress.h > @@ -21,6 +21,7 @@ struct zstd_data { > #ifdef HAVE_ZSTD_SUPPORT > ZSTD_CStream *cstream; > ZSTD_DStream *dstream; > + int comp_level; > #endif > }; > > diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c > index 49093b21ee2d..122ee198a86e 100644 > --- a/tools/perf/util/mmap.c > +++ b/tools/perf/util/mmap.c > @@ -295,15 +295,14 @@ int mmap__mmap(struct mmap *map, struct mmap_params *mp, int fd, struct perf_cpu > > map->core.flush = mp->flush; > > - map->comp_level = mp->comp_level; > #ifndef PYTHON_PERF > - if (zstd_init(&map->zstd_data, map->comp_level)) { > + if (zstd_init(&map->zstd_data, mp->comp_level)) { > pr_debug2("failed to init mmap compressor, error %d\n", errno); > return -1; > } > #endif > > - if (map->comp_level && !perf_mmap__aio_enabled(map)) { > + if (mp->comp_level && !perf_mmap__aio_enabled(map)) { > map->data = mmap(NULL, mmap__mmap_len(map), PROT_READ|PROT_WRITE, > MAP_PRIVATE|MAP_ANONYMOUS, 0, 0); > if (map->data == MAP_FAILED) { > diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h > index f944c3cd5efa..0df6e1621c7e 100644 > --- a/tools/perf/util/mmap.h > +++ b/tools/perf/util/mmap.h > @@ -39,7 +39,6 @@ struct mmap { > #endif > struct mmap_cpu_mask affinity_mask; > void *data; > - int comp_level; > struct perf_data_file *file; > struct zstd_data zstd_data; > }; > diff --git a/tools/perf/util/zstd.c b/tools/perf/util/zstd.c > index 48dd2b018c47..60f2d749b1c0 100644 > --- a/tools/perf/util/zstd.c > +++ b/tools/perf/util/zstd.c > @@ -7,35 +7,9 @@ > > int zstd_init(struct zstd_data *data, int level) > { > - size_t ret; > - > - data->dstream = ZSTD_createDStream(); > - if (data->dstream == NULL) { > - pr_err("Couldn't create decompression stream.\n"); > - return -1; > - } > - > - ret = ZSTD_initDStream(data->dstream); > - if (ZSTD_isError(ret)) { > - pr_err("Failed to initialize decompression stream: %s\n", ZSTD_getErrorName(ret)); > - return -1; > - } > - > - if (!level) > - return 0; > - > - data->cstream = ZSTD_createCStream(); > - if (data->cstream == NULL) { > - pr_err("Couldn't create compression stream.\n"); > - return -1; > - } > - > - ret = ZSTD_initCStream(data->cstream, level); > - if (ZSTD_isError(ret)) { > - pr_err("Failed to initialize compression stream: %s\n", ZSTD_getErrorName(ret)); > - return -1; > - } > - > + data->comp_level = level; > + data->dstream = NULL; > + data->cstream = NULL; > return 0; > } > > @@ -63,6 +37,21 @@ size_t zstd_compress_stream_to_records(struct zstd_data *data, void *dst, size_t > ZSTD_outBuffer output; > void *record; > > + if (!data->cstream) { > + data->cstream = ZSTD_createCStream(); > + if (data->cstream == NULL) { > + pr_err("Couldn't create compression stream.\n"); > + return -1; > + } > + > + ret = ZSTD_initCStream(data->cstream, data->comp_level); > + if (ZSTD_isError(ret)) { > + pr_err("Failed to initialize compression stream: %s\n", > + ZSTD_getErrorName(ret)); > + return -1; I'm not sure if the callers are ready to handle the failure. Thanks, Namhyung > + } > + } > + > while (input.pos < input.size) { > record = dst; > size = process_header(record, 0); > @@ -96,6 +85,20 @@ size_t zstd_decompress_stream(struct zstd_data *data, void *src, size_t src_size > ZSTD_inBuffer input = { src, src_size, 0 }; > ZSTD_outBuffer output = { dst, dst_size, 0 }; > > + if (!data->dstream) { > + data->dstream = ZSTD_createDStream(); > + if (data->dstream == NULL) { > + pr_err("Couldn't create decompression stream.\n"); > + return -1; > + } > + > + ret = ZSTD_initDStream(data->dstream); > + if (ZSTD_isError(ret)) { > + pr_err("Failed to initialize decompression stream: %s\n", > + ZSTD_getErrorName(ret)); > + return -1; > + } > + } > while (input.pos < input.size) { > ret = ZSTD_decompressStream(data->dstream, &output, &input); > if (ZSTD_isError(ret)) { > -- > 2.42.0.609.gbb76f46606-goog >
On Wed, Oct 18, 2023 at 4:21 PM Namhyung Kim <namhyung@kernel.org> wrote: > > On Wed, Oct 11, 2023 at 11:24 PM Ian Rogers <irogers@google.com> wrote: > > > > Zstd streams create dictionaries that can require significant RAM, > > especially when there is one per-CPU. Tools like perf record won't use > > the streams without the -z option, and so the creation of the streams > > is pure overhead. Switch to creating the streams on first use. > > > > Signed-off-by: Ian Rogers <irogers@google.com> > > --- > > tools/perf/util/compress.h | 1 + > > tools/perf/util/mmap.c | 5 ++-- > > tools/perf/util/mmap.h | 1 - > > tools/perf/util/zstd.c | 61 ++++++++++++++++++++------------------ > > 4 files changed, 35 insertions(+), 33 deletions(-) > > > > diff --git a/tools/perf/util/compress.h b/tools/perf/util/compress.h > > index 0cd3369af2a4..9391850f1a7e 100644 > > --- a/tools/perf/util/compress.h > > +++ b/tools/perf/util/compress.h > > @@ -21,6 +21,7 @@ struct zstd_data { > > #ifdef HAVE_ZSTD_SUPPORT > > ZSTD_CStream *cstream; > > ZSTD_DStream *dstream; > > + int comp_level; > > #endif > > }; > > > > diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c > > index 49093b21ee2d..122ee198a86e 100644 > > --- a/tools/perf/util/mmap.c > > +++ b/tools/perf/util/mmap.c > > @@ -295,15 +295,14 @@ int mmap__mmap(struct mmap *map, struct mmap_params *mp, int fd, struct perf_cpu > > > > map->core.flush = mp->flush; > > > > - map->comp_level = mp->comp_level; > > #ifndef PYTHON_PERF > > - if (zstd_init(&map->zstd_data, map->comp_level)) { > > + if (zstd_init(&map->zstd_data, mp->comp_level)) { > > pr_debug2("failed to init mmap compressor, error %d\n", errno); > > return -1; > > } > > #endif > > > > - if (map->comp_level && !perf_mmap__aio_enabled(map)) { > > + if (mp->comp_level && !perf_mmap__aio_enabled(map)) { > > map->data = mmap(NULL, mmap__mmap_len(map), PROT_READ|PROT_WRITE, > > MAP_PRIVATE|MAP_ANONYMOUS, 0, 0); > > if (map->data == MAP_FAILED) { > > diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h > > index f944c3cd5efa..0df6e1621c7e 100644 > > --- a/tools/perf/util/mmap.h > > +++ b/tools/perf/util/mmap.h > > @@ -39,7 +39,6 @@ struct mmap { > > #endif > > struct mmap_cpu_mask affinity_mask; > > void *data; > > - int comp_level; > > struct perf_data_file *file; > > struct zstd_data zstd_data; > > }; > > diff --git a/tools/perf/util/zstd.c b/tools/perf/util/zstd.c > > index 48dd2b018c47..60f2d749b1c0 100644 > > --- a/tools/perf/util/zstd.c > > +++ b/tools/perf/util/zstd.c > > @@ -7,35 +7,9 @@ > > > > int zstd_init(struct zstd_data *data, int level) > > { > > - size_t ret; > > - > > - data->dstream = ZSTD_createDStream(); > > - if (data->dstream == NULL) { > > - pr_err("Couldn't create decompression stream.\n"); > > - return -1; > > - } > > - > > - ret = ZSTD_initDStream(data->dstream); > > - if (ZSTD_isError(ret)) { > > - pr_err("Failed to initialize decompression stream: %s\n", ZSTD_getErrorName(ret)); > > - return -1; > > - } > > - > > - if (!level) > > - return 0; > > - > > - data->cstream = ZSTD_createCStream(); > > - if (data->cstream == NULL) { > > - pr_err("Couldn't create compression stream.\n"); > > - return -1; > > - } > > - > > - ret = ZSTD_initCStream(data->cstream, level); > > - if (ZSTD_isError(ret)) { > > - pr_err("Failed to initialize compression stream: %s\n", ZSTD_getErrorName(ret)); > > - return -1; > > - } > > - > > + data->comp_level = level; > > + data->dstream = NULL; > > + data->cstream = NULL; > > return 0; > > } > > > > @@ -63,6 +37,21 @@ size_t zstd_compress_stream_to_records(struct zstd_data *data, void *dst, size_t > > ZSTD_outBuffer output; > > void *record; > > > > + if (!data->cstream) { > > + data->cstream = ZSTD_createCStream(); > > + if (data->cstream == NULL) { > > + pr_err("Couldn't create compression stream.\n"); > > + return -1; > > + } > > + > > + ret = ZSTD_initCStream(data->cstream, data->comp_level); > > + if (ZSTD_isError(ret)) { > > + pr_err("Failed to initialize compression stream: %s\n", > > + ZSTD_getErrorName(ret)); > > + return -1; > > I'm not sure if the callers are ready to handle the failure. Thanks, fixed in v3. Ian > Thanks, > Namhyung > > > > + } > > + } > > + > > while (input.pos < input.size) { > > record = dst; > > size = process_header(record, 0); > > @@ -96,6 +85,20 @@ size_t zstd_decompress_stream(struct zstd_data *data, void *src, size_t src_size > > ZSTD_inBuffer input = { src, src_size, 0 }; > > ZSTD_outBuffer output = { dst, dst_size, 0 }; > > > > + if (!data->dstream) { > > + data->dstream = ZSTD_createDStream(); > > + if (data->dstream == NULL) { > > + pr_err("Couldn't create decompression stream.\n"); > > + return -1; > > + } > > + > > + ret = ZSTD_initDStream(data->dstream); > > + if (ZSTD_isError(ret)) { > > + pr_err("Failed to initialize decompression stream: %s\n", > > + ZSTD_getErrorName(ret)); > > + return -1; > > + } > > + } > > while (input.pos < input.size) { > > ret = ZSTD_decompressStream(data->dstream, &output, &input); > > if (ZSTD_isError(ret)) { > > -- > > 2.42.0.609.gbb76f46606-goog > >
diff --git a/tools/perf/util/compress.h b/tools/perf/util/compress.h index 0cd3369af2a4..9391850f1a7e 100644 --- a/tools/perf/util/compress.h +++ b/tools/perf/util/compress.h @@ -21,6 +21,7 @@ struct zstd_data { #ifdef HAVE_ZSTD_SUPPORT ZSTD_CStream *cstream; ZSTD_DStream *dstream; + int comp_level; #endif }; diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c index 49093b21ee2d..122ee198a86e 100644 --- a/tools/perf/util/mmap.c +++ b/tools/perf/util/mmap.c @@ -295,15 +295,14 @@ int mmap__mmap(struct mmap *map, struct mmap_params *mp, int fd, struct perf_cpu map->core.flush = mp->flush; - map->comp_level = mp->comp_level; #ifndef PYTHON_PERF - if (zstd_init(&map->zstd_data, map->comp_level)) { + if (zstd_init(&map->zstd_data, mp->comp_level)) { pr_debug2("failed to init mmap compressor, error %d\n", errno); return -1; } #endif - if (map->comp_level && !perf_mmap__aio_enabled(map)) { + if (mp->comp_level && !perf_mmap__aio_enabled(map)) { map->data = mmap(NULL, mmap__mmap_len(map), PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, 0, 0); if (map->data == MAP_FAILED) { diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h index f944c3cd5efa..0df6e1621c7e 100644 --- a/tools/perf/util/mmap.h +++ b/tools/perf/util/mmap.h @@ -39,7 +39,6 @@ struct mmap { #endif struct mmap_cpu_mask affinity_mask; void *data; - int comp_level; struct perf_data_file *file; struct zstd_data zstd_data; }; diff --git a/tools/perf/util/zstd.c b/tools/perf/util/zstd.c index 48dd2b018c47..60f2d749b1c0 100644 --- a/tools/perf/util/zstd.c +++ b/tools/perf/util/zstd.c @@ -7,35 +7,9 @@ int zstd_init(struct zstd_data *data, int level) { - size_t ret; - - data->dstream = ZSTD_createDStream(); - if (data->dstream == NULL) { - pr_err("Couldn't create decompression stream.\n"); - return -1; - } - - ret = ZSTD_initDStream(data->dstream); - if (ZSTD_isError(ret)) { - pr_err("Failed to initialize decompression stream: %s\n", ZSTD_getErrorName(ret)); - return -1; - } - - if (!level) - return 0; - - data->cstream = ZSTD_createCStream(); - if (data->cstream == NULL) { - pr_err("Couldn't create compression stream.\n"); - return -1; - } - - ret = ZSTD_initCStream(data->cstream, level); - if (ZSTD_isError(ret)) { - pr_err("Failed to initialize compression stream: %s\n", ZSTD_getErrorName(ret)); - return -1; - } - + data->comp_level = level; + data->dstream = NULL; + data->cstream = NULL; return 0; } @@ -63,6 +37,21 @@ size_t zstd_compress_stream_to_records(struct zstd_data *data, void *dst, size_t ZSTD_outBuffer output; void *record; + if (!data->cstream) { + data->cstream = ZSTD_createCStream(); + if (data->cstream == NULL) { + pr_err("Couldn't create compression stream.\n"); + return -1; + } + + ret = ZSTD_initCStream(data->cstream, data->comp_level); + if (ZSTD_isError(ret)) { + pr_err("Failed to initialize compression stream: %s\n", + ZSTD_getErrorName(ret)); + return -1; + } + } + while (input.pos < input.size) { record = dst; size = process_header(record, 0); @@ -96,6 +85,20 @@ size_t zstd_decompress_stream(struct zstd_data *data, void *src, size_t src_size ZSTD_inBuffer input = { src, src_size, 0 }; ZSTD_outBuffer output = { dst, dst_size, 0 }; + if (!data->dstream) { + data->dstream = ZSTD_createDStream(); + if (data->dstream == NULL) { + pr_err("Couldn't create decompression stream.\n"); + return -1; + } + + ret = ZSTD_initDStream(data->dstream); + if (ZSTD_isError(ret)) { + pr_err("Failed to initialize decompression stream: %s\n", + ZSTD_getErrorName(ret)); + return -1; + } + } while (input.pos < input.size) { ret = ZSTD_decompressStream(data->dstream, &output, &input); if (ZSTD_isError(ret)) {
Zstd streams create dictionaries that can require significant RAM, especially when there is one per-CPU. Tools like perf record won't use the streams without the -z option, and so the creation of the streams is pure overhead. Switch to creating the streams on first use. Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/util/compress.h | 1 + tools/perf/util/mmap.c | 5 ++-- tools/perf/util/mmap.h | 1 - tools/perf/util/zstd.c | 61 ++++++++++++++++++++------------------ 4 files changed, 35 insertions(+), 33 deletions(-)