Uploaded image for project: 'mdsal'
  1. mdsal
  2. MDSAL-170

Populate _value in IpAddressBinary union lazily

    XMLWordPrintable

Details

    • Bug
    • Status: Verified
    • Resolution: Done
    • None
    • None
    • None
    • None
    • Operating System: All
      Platform: All

    • 5970

    Description

      The lispflowmapping project switched using YANG modeled binary representation for IP addresses [0] internally in the southbound plugin, where it is rarely necessary to have access to string representation of the addresses. I did some profling before and after the change, to assess the gains in performance. To my surprise, code using the union of binary IPv4 and IPv6 (i.e., IpAddressBinary) suffered a serious performance regression, instead of improving.

      For example, we switched this code:

      public static IpAddress getIpAddressFromInetAddress(InetAddress address)

      { return IetfInetUtil.INSTANCE.ipAddressFor(address); }

      to this:

      public static IpAddressBinary getIpAddressBinaryFromInetAddress(InetAddress address)

      { return IpAddressBinaryBuilder.getDefaultInstance(address.getAddress()); }

      Using YourKit with sampling, for 20 minutes, the first takes ~2s total time, while the second takes ~150s. The reason for the regression is the constructor of the union generated code, which looks like this:

      public IpAddressBinary(Ipv4AddressBinary _ipv4AddressBinary)

      { super(); this._ipv4AddressBinary = _ipv4AddressBinary; this._ipv6AddressBinary = null; this._value = BaseEncoding.base64().encode(_ipv4AddressBinary.getValue()).toCharArray(); }

      public IpAddressBinary(Ipv6AddressBinary _ipv6AddressBinary)

      { super(); this._ipv6AddressBinary = _ipv6AddressBinary; this._ipv4AddressBinary = null; this._value = BaseEncoding.base64().encode(_ipv6AddressBinary.getValue()).toCharArray(); }

      Those 150s are spent in the encode() method. I assume this is part of the solution for MDSAL-131 [1], where I only reviewed the proposed solution patch for functionality, and not performance. Note that we don't have this issue with the base types Ipv4AddressBinary and Ipv6AddressBinary, their classes don't even contain an encode() method, only decode() for passing a string as an argument to the constructor.

      This defeats the purpose of using binary types in the first place. Is there a way to solve MDSAL-131 in a way that doesn't affect performance? I guess for the IPv4 and IPv6 binary base types the base64 encoding is only done when really necessary. Could that be done for the union too?

      Thanks,
      -Lori

      [0] https://git.opendaylight.org/gerrit/gitweb?p=lispflowmapping.git;a=blob;f=mappingservice/lisp-proto/src/main/yang/odl-inet-binary-types.yang;h=9706197c7b2f4481396af002fffa1efe13ad3b09;hb=HEAD
      [1] https://bugs.opendaylight.org/show_bug.cgi?id=5446

      Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          People

            rovarga Robert Varga
            ljakab Lori Jakab
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: