General report of memory leaks in code. Review source for missing deletes.

[hollasch]

[trevordblack]

There is a serious gotcha to consider.

Once the material class is added in chapter 8, a programming pattern emerges:

list[n] = new sphere(center, radius, new material);

In order to correctly delete all allocated memory, 1. The sphere needs to be deleted and 2. the material needs to be deleted.
There is a problem here in that for the above programming pattern, the only thing that stores the allocated material pointer is the sphere class.
This means that deallocation of the material will need to happen in sphere code, or through public access of the sphere's stored material pointer.
The goto programming pattern for this is to create a ~sphere() and delete the material when we're there.

This is all largely acceptable.

A big problem arises when you start dealing with material/geometry instancing.
Invariably, as scene sizes grow in complexity, you'll start indexing material instances.

Multiple spheres (and geometry in general) will share a material resource.
If the learning programmer naively calls delete on one of those spheres (and deletes the material allocation), the remaining geometry will all then try to access dellocated memory.
That's not great.

We can continue forward with deleting a material when it's married to it's geometry, but we might need to address this later in the series.

[hollasch]

Thus shared_ptr<material>. Automated sharing, default destructor handling, programmer only needs to think about allocation. In addition, the shared nature of materials is made explicit rather than implicit.

A standard alternate pattern that is often used is to allocate objects from a session-wide heap. Allocation happens once, and rather than doing individual instance deletes, you just free the session heap and done. However, this would be a non-standard, custom solution, and not really appropriate given the scope of this book.

[trevordblack]

Broadly, the codebase has shared and unique memory. I'll code up a shared_ptr branch and we can make a judgement call. For unique memory, do we want to use a raw pointer, or a unique_ptr? The pattern I see most often in RT codebases is raw pointers and shared_ptrs. All memory in InOneWeekend is memory that exists for the lifetime of the program. It wouldn't be unreasonable to push proper memory management off to NextWeek.

…

On Mon, Aug 19, 2019, 11:19 Steve Hollasch ***@***.***> wrote: Thus shared_ptr<material>. Automated sharing, default destructor handling, programmer only needs to think about allocation. In addition, the shared nature of materials is made explicit rather than implicit. A standard alternate pattern that is often used is to allocate objects from a session-wide heap. Allocation happens once, and rather than doing individual instance deletes, you just free the session heap and done. However, this would be a non-standard, custom solution, and not really appropriate given the scope of this book. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <https://github.com/RayTracing/InOneWeekend/issues/57?email_source=notifications&email_token=ACNZVKFDDLGNYYQQZLTIPETQFLP2JA5CNFSM4IGUMEWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4T3G3A#issuecomment-522695532>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACNZVKG3IOELPKQOTL6JPBTQFLP2JANCNFSM4IGUMEWA> .

[hollasch]

It wouldn't be unreasonable to push proper memory management off to NextWeek.

That's an excellent point. Can we get through RT1 with no memory allocations? We're already using a static array for objects.

[trevordblack]

This might be a failure of imagination, but I'm failing to see how we can have no memory allocations and keep the
list[n] = new sphere(center, radius, new material);
pattern (in a way that would be easy to understand).

[hollasch]

I was thinking along the lines of a sphere factory pattern that hands out constructed instances from a fixed array of statically-allocated spheres. An allocator method constructs the sphere and hands back a pointer to it. Or just hands back a pointer to the next sphere object, which the caller then initializes.

However, we have allocations for

• hitable
• sphere
• lambertian
• metal
• hitable_list

So that's just too much additional overhead and complexity to require the reader to grok. Could also do a hybrid approach where we just add deletes where we can, and use shared_ptr where necessary. The disadvantage is the reader must understand two patterns instead of one.

Probably the best approach is to use unique and shared pointers. We'd need to add some brief explanation in the text for readers who aren't that familiar with C++.