Avro
  1. Avro
  2. AVRO-1050

Avro PHP consumes too much memory due to code in io.php (AvroStringIO append_str)

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.6.0, 1.6.1, 1.6.2, 1.6.3
    • Fix Version/s: 1.7.0
    • Component/s: php
    • Labels:
    • Release Note:
      Fix memory growth issue with Avro PHP due to code in AvroStringIO.append_str.

      Description

      While attempting to encode large messages, our team found that the PHP run-time would exceed its memory limit and die. Profiling and use of memory_get_usage showed us that we were spending a lot of time in append_str and that the PHP process grew after every call to this method, resulting in the leak-like behavior that caused the process to grow and then crash. We rewrote append_str to use the string concatenation operator instead of the somewhat elaborate scheme in place today (str_split, followed by the copying of one byte at a time from the latter array to the final string) and the memory usage issues disappeared with no loss of functionality. We would like to provide the patch to fix this issue.

      1. AVRO-1050-2.diff
        2 kB
        A B
      2. AVRO-1050.diff
        0.7 kB
        A B

        Activity

        Hide
        A B added a comment -

        As I mentioned we have a fix in place as follows (patch will be forthcoming):

        Original code (io.php, class AvroStringIO):
        private function append_str($str)

        { $this->check_closed(); $ary = str_split($str); $len = count($ary); $this->buffer = array_merge($this->buffer, $ary); $this->current_index += $len; return $len; }

        Fixed code (io.php, class AvroStringIO):
        private function append_str($str)

        { $this->check_closed(); $this->buffer .= $str; $len = strlen($str); $this->current_index += $len; return $len; }
        Show
        A B added a comment - As I mentioned we have a fix in place as follows (patch will be forthcoming): Original code (io.php, class AvroStringIO): private function append_str($str) { $this->check_closed(); $ary = str_split($str); $len = count($ary); $this->buffer = array_merge($this->buffer, $ary); $this->current_index += $len; return $len; } Fixed code (io.php, class AvroStringIO): private function append_str($str) { $this->check_closed(); $this->buffer .= $str; $len = strlen($str); $this->current_index += $len; return $len; }
        Hide
        A B added a comment -

        Index: io.php
        ===================================================================
        — io.php (revision 1304457)
        +++ io.php (working copy)
        @@ -291,14 +291,13 @@

        • @returns integer count of bytes written.
          */
          private function append_str($str)
        • { - $this->check_closed(); - $ary = str_split($str); - $len = count($ary); - $this->buffer = array_merge($this->buffer, $ary); - $this->current_index += $len; - return $len; - }

          +

          { + $this->check_closed(); + $this->buffer .= $str; + $len = strlen($str); + $this->current_index += $len; + return $len; + }

        /**

        • Truncates the truncate buffer to 0 bytes and returns the pointer
        Show
        A B added a comment - Index: io.php =================================================================== — io.php (revision 1304457) +++ io.php (working copy) @@ -291,14 +291,13 @@ @returns integer count of bytes written. */ private function append_str($str) { - $this->check_closed(); - $ary = str_split($str); - $len = count($ary); - $this->buffer = array_merge($this->buffer, $ary); - $this->current_index += $len; - return $len; - } + { + $this->check_closed(); + $this->buffer .= $str; + $len = strlen($str); + $this->current_index += $len; + return $len; + } /** Truncates the truncate buffer to 0 bytes and returns the pointer
        Hide
        A B added a comment -

        Patch for this issue.

        Show
        A B added a comment - Patch for this issue.
        Hide
        A B added a comment -

        Attachment AVRO-1050.diff has the patch.

        Show
        A B added a comment - Attachment AVRO-1050 .diff has the patch.
        Hide
        A B added a comment -

        Improved patch with changes suggested by JJ (jawed@php.net) to eliminate type warnings within PHP.

        Show
        A B added a comment - Improved patch with changes suggested by JJ (jawed@php.net) to eliminate type warnings within PHP.
        Hide
        A B added a comment -

        Hello, any news or updates on this please? Do you find the patch acceptable for this issue?

        Show
        A B added a comment - Hello, any news or updates on this please? Do you find the patch acceptable for this issue?
        Hide
        Doug Cutting added a comment -

        The patch applies cleanly for me and tests pass after applying it. I will commit this soon unless someone objects.

        Show
        Doug Cutting added a comment - The patch applies cleanly for me and tests pass after applying it. I will commit this soon unless someone objects.
        Hide
        Doug Cutting added a comment -

        I committed this.

        Show
        Doug Cutting added a comment - I committed this.

          People

          • Assignee:
            A B
            Reporter:
            A B
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development