Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8259710

Inlining trace leaks memory

    XMLWordPrintable

    Details

    • Subcomponent:
    • Resolved In Build:
      b07

      Backports

        Description


        We see C-heap leaking, originating in C2:

        ```
        1434777 [0x00007f58214af461] stringStream::stringStream(unsigned long)+0x55
        1434778 [0x00007f5820c6db3e] Compile::PrintInliningBuffer::PrintInliningBuffer()+0x6c
        1434779 [0x00007f5820c724f1] GrowableArrayWithAllocator<Compile::PrintInliningBuffer, GrowableArray<Compile::PrintInliningBuffer> >::grow(int)+0x163
        1434780 [0x00007f5820c7160e] GrowableArrayWithAllocator<Compile::PrintInliningBuffer, GrowableArray<Compile::PrintInliningBuffer> >::insert_before(int, Compile::PrintInliningBuffer const&)+0x82
        1434781 [0x00007f5820c6aaf2] Compile::print_inlining_push()+0x70
        ```

        accumulating over the course of days to some hundred MB at a customer site where inline tracing was active.


        Analysis:


        To build up a comprehensive inlining trace, c2 keeps trace snippets in an internal list and assembles them at print time. These are stringStream's, contained in PrintInliningBuffer:

        ```
        GrowableArray<PrintInliningBuffer>* _print_inlining_list;
        ...
          class PrintInliningBuffer : public ResourceObj {
           private:
            CallGenerator* _cg;
            stringStream* _ss;

           public:
            PrintInliningBuffer()
              : _cg(NULL) {
              _ss = new stringStream();
            }

            void freeStream() {
              _ss->~stringStream(); _ss = NULL; }
        ...
          };
        ```

        With JDK-8224193, stringStream was changed to use C-heap instead of ResourceArea for its internal buffer. This means one cannot just omit stringStream destruction anymore - even where stringStream objects themselves live in RA, their internal buffers don't, they live in C-Heap. To clean up properly, ~stringStream() must be called.

        Those `PrintInliningBuffer` objects are kept _by value_ in a GrowableArray `Compile::_print_inlining_list`. Normally, if an object is kept by value, it needs to implement correct copy- and destruction-semantics. PrintInliningBuffer does not do that and instead relies on manual cleanup (`PrintInliningBuffer::freeStream()`) - I assume the authors did not want to deal with reference counting the contained stringStream on copying.

        That however is a problem because GrowableArray creates internally temporary objects of the item type to pre-populate its array - its whole capacity - when it grows:


        ```
        template <typename E, typename Derived>
        void GrowableArrayWithAllocator<E, Derived>::grow(int j) {

        ...
          for ( ; i < this->_len; i++) ::new ((void*)&newData[i]) E(this->_data[i]);
          for ( ; i < this->_max; i++) ::new ((void*)&newData[i]) E(); <<<<<<<
          for (i = 0; i < old_max; i++) this->_data[i].~E();
        ...
        }
        ```

        this it does for the whole new capacity, which means more objects get created than have been added; if the caller does not fill the GrowableArray up to its capacity, it never knows about the excess objects created. This is normally not a problem since all these objects are destructed by GrowableArray in `void GrowableArrayWithAllocator<E, Derived>::clear_and_deallocate()`. But since PrintInliningBuffer does not implement a destructor, this has no effect.

        PrintInliningBuffer instead relies on manual cleanup of the stringStreams: at the end of the compile phase, it calls `PrintInliningBuffer::freeStream()` on all buffers it thinks are contained in the array:

        ```
            assert(_print_inlining_list != NULL, "process_print_inlining should be called only once.");
            for (int i = 0; i < _print_inlining_list->length(); i++) {
              ss.print("%s", _print_inlining_list->adr_at(i)->ss()->as_string());
              _print_inlining_list->at(i).freeStream();
            }
        ```

        but this of course leaves the excess objects untouched (objects whose index is array length >= index >= array capacity).

          Attachments

            Issue Links

              Activity

                People

                Assignee:
                stuefe Thomas Stuefe
                Reporter:
                stuefe Thomas Stuefe
                Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                  Dates

                  Created:
                  Updated:
                  Resolved: