Avro
  1. Avro
  2. AVRO-1041

Utf8 allocates new byte array unnessisarily

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.6.2
    • Fix Version/s: 1.6.3
    • Component/s: java
    • Labels:
      None

      Description

      When a Utf8 instance is about to receive new data (i.e. in BinaryDecoder), Utf8::setByteLength is invoked to essentially ensure capacity of the backing byte array.
      However, the logical length of the current instance is compared against the required size rather than the existing byte array size.
      This causes needless allocations of a new backing byte array: If you read a 10 byte string followed by an 8 byte string followed by a 9 byte string, the 3rd read will cause a new backing array allocation even though the instance already has a 10 byte array at its disposal.
      At a minimum we should replace:

        public Utf8 setByteLength(int newLength) {
          if (this.length < newLength) {
            byte[] newBytes = new byte[newLength];
            System.arraycopy(bytes, 0, newBytes, 0, this.length);
            this.bytes = newBytes;
          }
          ...
        }
      

      with:

        public Utf8 setByteLength(int newLength) {
          if (this.bytes.length < newLength) {
            byte[] newBytes = new byte[newLength];
            System.arraycopy(bytes, 0, newBytes, 0, this.length);
            this.bytes = newBytes;
          }
          ...
        }
      

      We may also wish to consider setting a maximum size limit to the utf8 instance: If we allocate over this, we drop the backing array the next time we get a resize for a data length smaller than this (so we aren't forced to keep memory for the largest utf8 encountered in memory).

        Activity

        Hide
        dave irving added a comment -

        Patch just addresses resize bug.

        Show
        dave irving added a comment - Patch just addresses resize bug.
        Hide
        Doug Cutting added a comment -

        I committed this. Thanks, Dave!

        Show
        Doug Cutting added a comment - I committed this. Thanks, Dave!

          People

          • Assignee:
            dave irving
            Reporter:
            dave irving
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development