api: add pvt::heapsize() and pvt::footprint() methods and image cache memory tracking

[bfraboni]

Description

First PR of two, following @lgritz and I discussions on memory tracking in the OIIO::ImageCache.

• Add two template methods and their specializations for types various to help memory tracking:

// return the total heap allocated memory held by the object and its members
template<typename T> inline size_t heapsize(const T& t);
// return the total memory footprint including the size of the structure itself
template<typename T> inline size_t footprint(const T& t);

• Specialized for: ParamValue, ParamValueList, ImageSpec, ImageInput, ImageOutput, std::vector, std::shared_ptr, std::unique_ptr, oiio::intrusive_ptr, ImageCacheImpl related objects.
• New files:
  • include/memory.h : adds base and few types specialization of heapsize and footprint.
  • libtexture/imagecache_memory_pvt.h : adds specilializations for ImageCacheImpl and related objects.
  • libtexture/imagecache_memory_print.h : adds a helper function that print a breakdown of the ImageCacheImpl total memory usage as well, as well as per image format. Note: this is slow, but gives accurate memory estimation.

Related PR from Larry : #4317

Tests

• todo: fix CI fails

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: bfraboni / name: Basile Fraboni (fc60c39)

[lgritz]

I have an alternate design I'd like to suggest, that I think might have some benefits.

We start with the fully general template declaration that from a caller's point of view we use for everything:

/// Return the total memory footprint of `object`. If possible, including any heap
/// allocations done by any constituent parts. The default implementation just reducees
/// to sizeof(object), but template specialization can be used to give improved results
/// for tricky types of those that perform heap allocation.
template<typename T>
size_t footprint(const T& object) noexcept
{
    // The base case / fallback is equivalent to sizeof(T)
    return sizeof(object);
}

Everything else is defined by specialization:

// Specialization that does std::string correctly
template<>
size_t footprint(const std::string& str) noexcept
{
    size_t size = sizeof(str);
    if (!is_short_string(str) /* implementation TBD */)
        size += str.capacity();
    return size;
}

// Specialization that does vectors correctly
template<typename T>
size_t footprint(const std::vector<T>& vec) noexcept
{
    size_t size = sizeof(vec);
    for (const auto& e : vec)
        size += footprint(e);
    size += (vec.capacity() - vec.size()) * sizeof(T);
    return size;
}

// Specialization that does arrays correctly even for nontrivial types
template<typename T, size_t N>
size_t footprint(const T (&arr)[N]) noexcept
{
    size_t size = 0;
    for (const auto& e : arr)
        size += footprint(e);
    return size;
}

we will think of more specializations and add as needed.

Then we add for our own types:

// in paramlist.h:
template<> size_t footprint(const ParamValueList& pl);

// in paramlist.cpp:
template<> size_t footprint(const ParamValueList& pl)
{
    ... implementation ...
}

We don't really need ParamValueList::footprint(), though if we really want, we could add the method and either define it as a call to the template, or define the template to call the method, whatever we prefer.

But this also would allow us in the future to easily add footprint ability to classes that maybe we couldn't add it as a method without breaking ABIs, because it would actually be outside the class. And then we could add the convenience method later, when it's a point that we are comfortable breaking interfaces.

By defining it to look like a single uniform template, that means that anywhere in the code, we could just ask for footprint(thing) and it would just work -- you don't need to pick one of several names depending on the type of data structure that thing is. You don't even really need to know if there's a footprint method on the object, because it will always fall back to at least giving you the size of the base object itself, even if it doesn't fully account for any dynamic allocations hanging off of it (and you can add the specialization later).

[bfraboni]

I like that template suggestion @lgritz , I can do that but keep datasize() as follows:

template<typename T>
size_t datasize(const T& object) noexcept
{
    // The base datasize() is always 0
    return 0;
}

/// 
/// specialize datasize() for every required object
/// 

// Specialization that does objects and pointers correctly
template<typename T>
size_t footprint(const T& object) noexcept
{
    if constexpr (is_pointer<T>::value)
    {
        return sizeof(T) + (object ? footprint(*object) : 0);
    }

    // The base footprint() is always sizeof(T)
    return sizeof(T) + datasize(object);
}

I use the vector_datasize variants for std::vector and std::vector<T*> a lot, and never used a "footprint" counterpart, especially in the other PR.
And what you suggest size_t size = sizeof(vec); would double count the sizeof vector if it is a member variable of an object, so I still feel it is worth having datasize() for every objects and vector / array types, populate the datasize() correctly using the members datasize() (or footprint() for vector allocations) and having a trivial footprint() function.

Would that make sense ?

[lgritz]

I like that template suggestion @lgritz , I can do that but keep datasize() as follows:

Yeah, I'm ok with that. But let's change it to heapsize() or allocsize() or something more clear so that it (a) doesn't conflict with the existing datasize() name, which we already use in ParamValue and that represents the... data size... and not the allocation size; and (b) is more self-documenting about what it actually means, and has a rigorous definition to go along with it. That definition should be clear about how it should behave in "short string" or "short vector" (which we don't use yet, but I'd like to in the future) situations.

May I suggest the following definitions?

foo::datasize() -- how big the valid data is, having nothing to do with size or location of allocation. That is, it's how big you'd need to malloc or memcpy to get the relevant data representation. Only some types have/need this.

footprint(obj) -- as best as we can determine, the TOTAL memory footprint of object and everything obj owns. Should be usable with any object, ours or std.

heapallocsize(obj) -- total size of any heap allocation within obj. (Is it recursive, does it include heap allocs of things contained in things that obj allocated?) I think that this in general should be equal to footprint(obj) - sizeof(obj), but I'm not certain I understand a consistent definition for the recursive parts.

[lgritz]

I'm still not sure I know what datasize() (or heapsize or whatever) should return in all cases. If obj contains a std::string, should datasize(obj) count the heap allocations of the string, also? What about recursive objects? Let me make an example:

struct A {  // pretend sizeof(A) == 32+32+4
    std::string s;    // might have a heap alloc, if the string is big
    std::vector<char> vc;  // has a heap alloc if not empty
    int i;
};

struct B {   // pretend sizeof(B) = 4+32+sizeof(A)
    float f;
    std::string s;  // might have a heap alloc if big
    A a;
};

B b;

I think we agree that

footprint(b) = sizeof(B) + b.s.heapsize + b.a.s.heapsize + b.a.vc.heapsize

but what is heapallocsize(b) (or, in your terms, datasize(b))?

[bfraboni]

I'd write the duo footprint / heapallocsize the following way:

struct A {  // pretend sizeof(A) == 32+32+4
    std::string s;    // might have a heap alloc, if the string is big
    std::vector<char> vc;  // has a heap alloc if not empty
    int i;
};

struct B {   // pretend sizeof(B) = 4+32+sizeof(A)
    float f; // trivial
    std::string s;  // might have a heap alloc if big
    A a; // might have heap alloc
};

// default heapallocsize
template <typename T> size_t heapallocsize(const T& ref) { return 0; }

// heapallocsize specialization for std::string
template <> size_t heapallocsize<std::string>(const std::string& s) { return s.is_small ?  0 : s.capacity(); }

// default footprint for object or ptr to object
template <typename T> size_t footprint(const T& t)
{ 
    return std::is_pointer(T)::value ? 
        sizeof(T) + heapallocsize(t) : 
        sizeof(T) + footprint(*t); 
}

// heapallocsize specialization for std::vector
template <typename T> size_t heapallocisze(const std::vector<T>& v)
{ 
    size_t size = 0;
    for (const auto& e : vec)
        size += footprint(e);
    return size;
}

// heapallocsize specialization for A
// - counts the heapallocsize of all member that potentially alloc on the heap, so the string and the vector
// - the rest is automatically accounted for in "footprint" 
template <> size_t heapallocsize<A>(const A& a) { return heapallocsize(a.s) + heapallocsize(a.vc); }

// heapallocsize specialization for B
// - counts the heapallocsize of all member that potentially alloc on the heap, so the string and the structure A
// - the rest is automatically accounted for in "footprint" 
template <> size_t heapallocsize<B>(const B& b) { return heapallocsize(b.s) +heapallocsize(b.a) }; 

The only work to do is for every structure we want to be able to track, list the internal members that have potential heap allocations and specialize heapallocsize !

I agree with all definitions, and sorry about the confusion, I didn't count the allocation of ParamValues correctly and I misused datasize(); will fix that too..

[lgritz]

The approach looks fine to me, we're very close, though as I noted, I think you can keep the minimal declarations in the headers and move most of the specialized implementations into the modules where the rest of those class methods are implemented. I think that's the only big suggestion I have at this point.

[lgritz]

Does it not work to declare only the specialized one as a friend here? Like

template<> friend size_t pvt::heapsize(const ParamValue&);

[bfraboni]

I think when I've tried it didn't, but I can give it another go , just in case !

[bfraboni]

Indeed, it doesn't work: "error: cannot declare an explicit specialization in a friend"

[bfraboni]

I implemented your suggestions in the latest commit @lgritz , it should be closer now, all these make a lot of sense 👍
I also rebased to master, squashed commits to one to get clean history, and formatted with clang-format.

[lgritz]

Something seems unhappy in the CI.

[bfraboni]

Something seems unhappy in the CI.

Sorry about that, I haven't properly tested the latest version.
Should be fine now @lgritz, I fixed the compilation issue and tests pass locally !

[lgritz]

This all looks great!

I did have one question about why you made one member mutable. I'll wait to merge until you have a chance to respond, just in case that was inadvertent.

[bfraboni]

@lgritz I did that change to be able to loop through TileCache from a const reference to ImageCacheImpl, because unordered_map_concurrent doesn't implement const iterators. Maybe not the wiser choice, though ?

[lgritz]

It's all a private type, not part of public APIs, so I think it's fine. Let's leave it, but perhaps you could put a comment next to that declaration that just says "FIXME: if unordered_map_concurrent had const iterators, this wouldn't need to be mutable"?

[bfraboni]

Just added the comment!

[lgritz]

LGTM!