Skip to content

Optimize and reduce memory usage of XML serialization #14204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 11, 2024

Conversation

nielsdos
Copy link
Member

The serialization process uses the system allocator and requires a copy to request allocated memory once finished. This patch improves this by using smart_str to build the resulting string, reducing the number of copies and reducing total peak memory usage.

The serialization process uses the system allocator and requires a copy
to request allocated memory once finished. This patch improves this by
using smart_str to build the resulting string, reducing the number of
copies and reducing total peak memory usage.
@devnexen
Copy link
Member

How looks like the perf improvement ? :)

Comment on lines +253 to +258
static int php_new_dom_write_smart_str(void *context, const char *buffer, int len)
{
smart_str *str = context;
smart_str_appendl(str, buffer, len);
return len;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use size_t instead of int?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The interface is defined by libxml, so it has to be int.

@nielsdos
Copy link
Member Author

nielsdos commented May 11, 2024

How looks like the perf improvement ? :)

For 100 serializations:

[niels@ARCH RELx64]$ hyperfine './sapi/cli/php xml.php' './sapi/cli/php_old xml.php'
Benchmark 1: ./sapi/cli/php xml.php
  Time (mean ± σ):     258.7 ms ±   5.9 ms    [User: 250.2 ms, System: 6.6 ms]
  Range (min … max):   252.1 ms … 267.9 ms    11 runs
 
Benchmark 2: ./sapi/cli/php_old xml.php
  Time (mean ± σ):     319.7 ms ±  19.1 ms    [User: 281.9 ms, System: 36.1 ms]
  Range (min … max):   301.9 ms … 368.9 ms    10 runs
 
Summary
  ./sapi/cli/php xml.php ran
    1.24 ± 0.08 times faster than ./sapi/cli/php_old xml.php

As for memory usage:

Before: Maximum resident set size (kbytes): 25880
After: Maximum resident set size (kbytes): 24568

Test case used: https://gist.github.com/nielsdos/369813d1b1c5c146a6fd7992b8ddbc28

Copy link
Member

@devnexen devnexen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MSTM

@nielsdos nielsdos merged commit aa3e6ee into php:master May 11, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants