Change structEntry allocation scheme

[VisenDev]

After some though on structEntry allocations, I have concluded thus: Instead of having two modes, alloc and non-alloc, we should instead have three allocation modes

• None/Prealloc: Dvui does not allocate any memory, pointers and slices are assumed to be preallocated by the application. Any slice field's initial length is assumed to be identical to capacity. Slice length and capacity cannot change.
• Temporary/GarbageCollected: Dvui allocates memory in temporary dataGet buffers. Users must copy any data allocated this way to avoid it being freed automatically by dvui.
• Persistent/Leaky: Dvui accepts an allocator parameter which it uses to allocate all memory. The application is responsible for freeing any of this memory.

[david-vanderson]

My spider sense in tingling here - this seems like we might be heading down the wrong abstraction. It's unclear to me what situation an app would be in where it would want dvui to allocate things for it.

We should try to write some sample code that would use this api before trying to make it work. Can you give an example usecase with sample code for each situation?

[VisenDev]

My spider sense in tingling here - this seems like we might be heading down the wrong abstraction. It's unclear to me what situation an app would be in where it would want dvui to allocate things for it.

Just in our single example of editing the current theme we need this. We can't really rely on the font name text buffers having any extra capacity or even being mutable memory. If we want to be able to modify any text fields we need to allocate new memory or set up the currentTheme beforehand to have extra memory.

If we didn't want dvui to allocate new memory in structEntry, we would have to create a new a new "mutable copy" dvui.Theme and allocate a large capacity for each slice field. Then we would need to copy all fields from the current theme into our "mutable copy" theme. Then we would need to set the current theme to be the mutable copy.

Relying on the application preallocating memory is fine for small structs. But if you want to edit something like a dvui.Theme with dozens of sub structs containing text buffer, that seems like it would be inconvenient and impractical.

Another problem is that I dont think we can edit []const u8 fields (or really any cost pointer fields) if we rely on preallocated memory. If dvui allocated the memory inside struct entry this would not be a problem since we can change where the [] const u8 points to.

We should try to write some sample code that would use this api before trying to make it work. Can you give an example usecase with sample code for each situation?

• None: Small structs with only a few pointer fields. These fields can easily be manually allocated with the proper capacities

None Alloc Example

const Foo = struct {
     a: *usize,
     b: []u8,  //note if these were const pointer fields this allocation type wouldn't work since we couldn't edit their values
};

var my_foo = Foo{
   .a = try alloc.create(usize),
   .b = try alloc.alloc(u8, 64),
};

try dvui.structEntry(@src(), Foo, &my_foo, .{ });

• Temporary: The same functionality can also be achieved with persistent allocations so this method can be removed. I only included because that is how I am handling new allocations at the moment. Persistent allocations are probably better.

• Persistent: Any structs that you want memory to be allocated for automatically using an allocator parameter - this would operate in the same way as parsing a struct from json allocates the memory automatically. std.json would be very unpleasant to use if you had to manually allocate the result type and make sure every single field had enough capacity to hold any parsed data.

One use for persistent allocations, as described above, is editing a theme. If we want to change a font name we need to allocate new memory. It would feel pretty natural to me to just pass in an explicit allocator parameter which will allocate memory for any pointer or slice fields.

Any system which needs to parse unknown data into a struct will have this problem. std.json handles this problem by passing in an explicit allocator parameter.

For an example of a parsing interface that uses the sort of system I am proposing, you can look at the toAny and toAnyAlloc functions in ziglua which I worked on a few months back https://github.com/natecraddock/ziglua/blob/74796808b5bf7c81bf6f1e1b48d97ad01f87ec45/src/lib.zig#L3125

Persistent alloc example

const Foo = struct {
     a: *const usize,
     b: []const u8, 
     c: []const u8,
     d: []const u8,
     e: []const u8,
     f: []const u8,
     g: []const u8,
     h: []const u8,
     i: []const u8,
     j: []const u8,
     k: []const u8,
     l: []const u8,
};

const Bar = struct {
    foos: []const Foo,
    bip: ?*const Bar,
    bar: ?*const Bar,
};

var my_bar: Bar = undefined;

// We need to have dvui allocate the memory here since allocating it manually would be impractical
// This allows dvui to allocate more memory for slice fields if needed
try dvui.structEntryAlloc(@src(), my_arena_allocator, Bar, &my_bar, .{ });

//use my_bar as needed

// free all memory once we are done with it
my_arena_allocator.free();

I hope this helps explain my thought process here

[david-vanderson]

Yes this helps a lot. I don't think the parsing example fits here, because here you already have to have a struct in order to call the function.

But I think you are on the right track with the theme name editing. Let's focus first on that and see how far we get. The 2 cases are "None" and "Alloc".

First problem is slices (just thinking of []u8 here):

• If the slice is to constant memory:
  • None: we render a label (or textLayout) instead of textEntry
  • Alloc: we use dataGetDefault behind the scenes, and if the value changes, we alloc a new slice (caller owns it)
• If the slice is mutable
  • None: directly pass slice to textEntry
  • Alloc: directly pass slice to textEntry
• In both Alloc cases, we need to deal with increasing or decreasing the size of the buffer to textEntry. Maybe textEntry itself needs to be able to do this (at least increase the buffer size with some limit)?

Does this make sense?

[VisenDev]

Yes this helps a lot. I don't think the parsing example fits here, because here you already have to have a struct in order to call the function.

Okay that is a fair point, a parsing function is called once to parse some data. Our function needs to be called repeatedly

• If the slice is to constant memory:

  • None: we render a label (or textLayout) instead of textEntry
  • Alloc: we use dataGetDefault behind the scenes, and if the value changes, we alloc a new slice (caller owns it)

• If the slice is mutable

  • None: directly pass slice to textEntry
  • Alloc: directly pass slice to textEntry

Okay I think this is a good strategy. I will work on writing something up to explain how each memory allocation situation should be handled.

• In both Alloc cases, we need to deal with increasing or decreasing the size of the buffer to textEntry. Maybe textEntry itself needs to be able to do this (at least increase the buffer size with some limit)?

I think my rough plan to handle allocating text entries without changing their implementation was going to be 1) initially allocate a default sized buffer for the text entry 2) In every iteration, before calling the text entry, if we see that more than half the buffer is used by the text entry - we realloc the buffer and double its capacity.

But being able "request" more memory inside the textEntry somehow would be more ergonomic. I'm not super familiar with the internals of how the text buffer is used so I'm not sure how this should be done.

[iacore]

But being able "request" more memory inside the textEntry somehow would be more ergonomic. I'm not super familiar with the internals of how the text buffer is used so I'm not sure how this should be done.

Like this?

const Foo = struct {
     a: usize, // no indirection
     b: std.ArrayList(u8), // allocate enough capacity first
};

How much "more memory" do you need?

I feel like structEntry should be in utils/contrib, not part of the core library.

Here's an example of multiple fields: https://git.envs.net/iacore/porkbun-gui-rs/src/commit/c75ab94ce0d01e3b23bbfcffb1552fd60b3496b4/src/main.rs#L86-L92

It looks like this:

text field password text field password text field check box

How do you tell between normal text and password text from type itself?

[david-vanderson]

That sounds great. The "expand if more than half is used" strategy I think works in most cases, but maybe not if the user tries to paste a large amount of text. I'm imagining a database viewer.

I'll try to add allocation with growable backing to textEntry, so go ahead assuming that will work.

@iacore That's interesting. Let me try adding the functionality and thinking about what the api should look like.

Generally I'm against having a in/out split. If something is generally useful I'm inclined to put it in, and if not leave it out. But I haven't thought about this much. What is the benefit of something being designated as "ouside the core" but still in the same git repo?

I believe that kind of thing is what the StructFieldOptions is for - giving extra detail about specific fields.

[VisenDev]

@david-vanderson Proposed memory handling scheme

For handling immutable pointers in no-alloc mode, the following field will be added to PointerFieldOptions(T) to determine how immutable fields are handled. Users can change the default if preferred.

immutable_field_behavior: enum {hide, display} = .hide,

Slice of u8

• Const Pointer
  • None: Use behavior specified by immutable_field_behavior
  • Alloc: Use the provided explicit allocator parameter to alloc and realloc new text buffers as needed.
• Mutable Pointer
  • None: Pass the result text buffer directly to textEntry
  • Alloc: Dvui manages allocating the text buffer. Always assume result text buffer pointer is undefined when initially passed in. Dvui will then allocate memory and assign it to this buffer - reallocating if needed.

Slice

• Const Pointer
  • None: Use behavior specified by immutable_field_behavior
  • Alloc: Use the provided explicit allocator parameter to alloc and expand the slice capacity as needed. New items can be added to the slice and rearranged using the reorderable list widget.
• Mutable Pointer
  • None: Assume initial result slice len is equal to result slice capacity. Do not change slice length. Use reorderable list widget to rearrange len number of items in the list.
  • Alloc: Use the provided explicit allocator parameter to alloc and expand the slice capacity as needed. New items can be added to the slice and rearranged using the reorderable list widget. Always assume initial result slice value when is undefined and needs to be allocated. (dvui will track whether it has initially allocated memory or not using the dataGet api)

One Item Pointer

• Const Pointer
  • None: Use behavior specified by immutable_field_behavior
  • Alloc: Use the provided explicit allocator parameter to alloc one item. Always assume initial result pointer value is undefined and needs to be allocated.

• Mutable Pointer

  • None: Assume pointer memory has already been allocated.
  • Alloc: Use the provided explicit allocator parameter to alloc one item. Always assume initial result pointer value is undefined and needs to be allocated.

  Many Item Pointer

• Forbidden because tracking allocated space is difficult and many item pointers are very rarely used

  C Style Pointer

• Forbidden because tracking allocated space is difficult and *C pointers should only appear in translateC code

[david-vanderson]

Seems reasonable, but I'm having enough trouble just thinking through []u8 so I'm going to stick with that for now.

The default for immutable_field_behavior should be .display - the primary usecase I'm thinking of for structEntry is you are running an app/game and you want to pop up a subwindow and show/edit everything in a particular variable (say the player struct in a game). Every field should be shown, even if we don't yet support editing (or even viewing) it.

I'm at a lost for how to deal with undefined. My guess is you are imagining allocating a struct, setting all the fields to undefined, and then using structEntry to modify it? How would structEntry tell the difference between a field that's undefined and the following frame where it is now properly defined? We could assume it's undefined on the first frame, but then how do you pass a struct that already has all the fields defined?

[VisenDev]

The default for immutable_field_behavior should be .display - the primary usecase I'm thinking of for structEntry is you are running an app/game and you want to pop up a subwindow and show/edit everything in a particular variable (say the player struct in a game). Every field should be shown, even if we don't yet support editing (or even viewing) it.

Oh thats a good point, I was also thinking structEntry could be useful for the dvui debugging window (like we could add a way to overwrite a widgets options and see how the widget behaves in real time)

I'm at a lost for how to deal with undefined. My guess is you are imagining allocating a struct, setting all the fields to undefined, and then using structEntry to modify it? How would structEntry tell the difference between a field that's undefined and the following frame where it is now properly defined? We could assume it's undefined on the first frame, but then how do you pass a struct that already has all the fields defined?

Let me give an example to help clarify what I mean when I say assume undefined

const Foo = struct {
    a: []const u8,
    b: ?*Foo,
 };

 var my_foo: Foo = undefined;

// ...

 _ = try dvui.structEntryAlloc(@src(), Foo, &my_foo, my_allocator, .{});

// ...

 //inside pointerFieldWidget

 //we keep track of the address where we have allocated the memory and allocate it if it has not been allocated by us.
const memory_ptr =  dvui.dataGet(null, id, "memory_ptr", *Foo);
if(memory_ptr == null) {
    dvui.dataSet(null, id, "memory_ptr", try allocator.create(Foo),);
}
// assign value to result.*.* using fieldWidgets

result.* = memory_ptr.?;

If the user wants to provide initial values for pointer fields, they should use non allocating structEntry.

Alternatively, in alloc mode, we could always assume that initially a pointer field is defined, and then copy what the pointer points to into our newly allocated memory.

Either way, what I was trying to acheive is

• For alloc mode: All memory is allocated with the provided allocator. Do not rely on any fields being preallocated.
• For none mode: All memory is assumed to be pre-allocated. All pointers are assumed to be valid.

For alloc mode, It just depends whether we want to a) always assume pointer fields initially point to valid memory and then copy the value of this memory when we initially allocate. Or b) always assume pointer fields initially are undefined and do not point to valid memory.

[david-vanderson]

I'm getting tripped up because there's a third thing we want. In alloc mode, a struct could have a []u8 that has already been preallocated, but we want to be able to realloc that slice if the text grows.

I like the code example, but I think it's still missing something:

const memory_ptr =  dvui.dataGet(null, id, "memory_ptr", *Foo);
if(memory_ptr == null) {
    dvui.dataSet(null, id, "memory_ptr", try allocator.create(Foo),);
}

// now I want to call textEntry on Foo.a, but what do I give it?  memory_ptr.?.a is undefined, so we'd need to zero it out first - same with result.a

// assign value to result.*.* using fieldWidgets

Can you put the code to call textEntry in there so I understand what slice of bytes you want to pass to it?

[VisenDev]

Can you put the code to call textEntry in there so I understand what slice of bytes you want to pass to it?

I was planning on doing something along the lines of this for the []const u8 field

var buffer_ptr =  dvui.dataGet(null, id, "buffer_ptr", []u8);
var buffer_capacity =  dvui.dataGet(null, id, "buffer_capacity", usize);
if(buffer_ptr == null) {
    const default_buffer_size = 128;
    dvui.dataSet(null, id, "buffer_ptr", try allocator.alloc(u8, default_buffer_size);
    buffer_ptr =  dvui.dataGet(null, id, "buffer_ptr", []u8);

     dvui.dataSet(null, id, "buffer_capacity", default_buffer_size);
     buffer_capacity =  dvui.dataGet(null, id, "buffer_capacity", usize);

    @memset(buffer_ptr, 0);
}

try dvui.textEntry(@src(), .{ .text = buffer_ptr.? }, .{});

//I'm not sure familiar with the internals to textEntry so I'm not sure how to figure out the "text_entry_text_len"
if(text_entry_text_len > buffer_capacity.? / 2) {
    dvui.dataSet(null, id, "buffer_ptr", try allocator.realloc(u8, buffer_ptr.?, buffer_capacity.? * 2);
    dvui.dataSet(null, id, "buffer_capacity", buffer_capacity.? * 2);
}

result.* = dvui.dataGet(null, id, "buffer_ptr", []u8).?;

[david-vanderson]

Okay good, then how would that code deal with a struct where it already had a preallocated slice? In that case on the first frame buffer_ptr will still be null, so we'd zero out whatever was preallocated, right?

[VisenDev]

Okay good, then how would that code deal with a struct where it already had a preallocated slice? In that case on the first frame buffer_ptr will still be null, so we'd zero out whatever was preallocated, right?

(Also If the user wanted us to actually use the preallocated slice to store our memory in, they should use the non-alloc mode. Managing any preallocated heap memory is up to the user for alloc mode)

If I understand you correctly that would depend on how we answer this question.

For alloc mode, It just depends whether we want to a) always assume pointer fields initially point to valid memory and then copy the value of this memory when we initially allocate. Or b) always assume pointer fields initially are undefined and do not point to valid memory.

If we go with option a) (assume initial pointers are valid) I would change the above code to do something like this instead

if(buffer_ptr == null) {
    const default_buffer_size = @max(result.*.len * 2, 128);
    dvui.dataSet(null, id, "buffer_ptr", try allocator.alloc(u8, default_buffer_size);
    buffer_ptr =  dvui.dataGet(null, id, "buffer_ptr", []u8);

     dvui.dataSet(null, id, "buffer_capacity", default_buffer_size);
     buffer_capacity =  dvui.dataGet(null, id, "buffer_capacity", usize);

     @memset(buffer_ptr, 0);
     std.mem.copyForwards(u8, buffer_ptr, result.*);
}

[david-vanderson]

Ah that helps a lot thank you. I'm still wondering how to support a preallocated slice that the user wants us to grow as needed. My hunch is that we want to assume pointer fields initially point to valid memory, otherwise structEntry will have to know how to create and initialize them. But I'm going to wait until we try to make an functioning example.

I just committed enhancements to TextEntryWidget to let it help manage the memory for you. Take a look and see if that helps at all.

[VisenDev]

Ah that helps a lot thank you. I'm still wondering how to support a preallocated slice that the user wants us to grow as needed.

I was planning to explicitly disallow this, because we don't know how, and with what allocator any preallocated slices were created. If you want structEntry to manage reallocing a slice, structEntry should be the one who initially allocated it.

This way we know the structEntryAlloc allocator parameter is the same allocator that our result slice was initialized with. (ie, we don't want to try freeing or reallocing a slice with the wrong allocator)

However, if there seems to be an ergonomic way to handle this problem we can work on adding support.

My hunch is that we want to assume pointer fields initially point to valid memory, otherwise structEntry will have to know how to create and initialize them.

That seems like a reasonable default. Then we can copy any values from that valid memory into our new memory.

For the example of editing the active dvui.Theme font field name using a structEntryAlloc, this would happen 1) The first time the function is called we check we have allocated memory already 2) We have not allocated memory, so we allocate a []u8 slice of a default size 3) We copy whatever value the font field initially had into our new slice 4) We assign the new slice to the font field

This approach would let us get the active value from the current theme, while not trying to edit immutable const memory.

Possible shortcomings

• If the dataGet data is cleared, we would lose our "handle" on the slice memory location. This means we would try to allocate a new slice and not free the old one - resulting in a leaked memory address.

Possible fix

• Implement a dataGet mode that does not clear unused data. This would prevent us from losing our "handle" to the slice memory location

But I'm going to wait until we try to make an functioning example.

Yes, I think we are good to implement our rough strategy here, and then modify it after testing.

I just committed enhancements to TextEntryWidget to let it help manage the memory for you. Take a look and see if that helps at all.

Awesome! I'll give that a look.

Sorry if this memory allocation discussion seems overly-complex. Properly handling memory in this situation is inherently difficult in a manually managed memory language. This is one of the few times a garbage collector would actually be really nice haha

[david-vanderson]

I was planning to explicitly disallow this, because we don't know how, and with what allocator any preallocated slices were created. If you want structEntry to manage reallocing a slice, structEntry should be the one who initially allocated it.

Argh you're right. And I'm starting to understand how you're thinking - any preallocated []u8 slice can be mutated in place, but even swapping it out for a new one we alloc could cause problems, so you're thinking we either mutate in place or have structEntry allocate everything (but then what if it's passed a struct that has some preallocated stuff?).

I don't see a way around this, so I'm tempted to say we need a simplifying assumption:

In the alloc case, any []u8 slices are assumed to have been allocated by the passed allocator, which means we can realloc at will.

In the case of trying to initialize a new struct, I think that works, because you'd use the same allocator to initialize as you pass to structEntry. There's an added bonus I found when doing the new textEntry allocation stuff - you can initialize a []u8 slice to be empty (field: []u8 = &.{}) and then any allocator frees that (as a no-op).

What do you think?

[VisenDev]

In the alloc case, any []u8 slices are assumed to have been allocated by the passed allocator, which means we can realloc at will.

This might work, however here are some possible issues.

• []const u8 and []u8 would have to be treated differently. We will always need to alloc our own []const u8, so the user needs to make sure any initially allocated memory is freed. However, for a []u8, we will be reallocing the initial memory so the user will break things if they try to free it.
• How would we handle the case of pointers being initialized without using an allocator (eg, taking the address of static buffer)? (Perhaps we could forbid doing this in alloc mode? There isn't really a point of trying to initialize things with a static buffer when you are going to dynamically allocate memory for it. Users who wanted to initialize pointers this way would probably be using the non-alloc mode, no?)

[david-vanderson]

I might be thinking wrongly, but I figured we would treat []const u8 as an immutable field?

I think you are right - let's try moving forward with the assumption that all allocations in alloc mode are from the same allocator.

That at least gets us your Foo example from above:

const Foo = struct {
    a: []const u8,
    b: ?*Foo,
 };

You can create one, and then structEntry can give the option of allocating an entry for b.

I'm thinking that non-alloc mode is useful for exploring existing data, possibly changing some values. Alloc mode is useful for populating new data (in temporary memory say), but is likely to require some kind of copying after.

[david-vanderson]

I might be thinking wrongly, but I figured we would treat []const u8 as an immutable field?

Ignore this, I was confused. If the field is []const u8 (and the user modifies it), then we allocate and the app now owns that memory. If it is []u8, we can free/allocate at will. So in the []const u8 case, if that memory was originally allocated and needs to be freed (so not pointing to constant strings) then the app has to deal with that.

I suspect that will be the rare case, so let's proceed and see how it falls out.

[VisenDev]

I suspect that will be the rare case, so let's proceed and see how it falls out.

Sounds good, thanks for clarifying

[iacore]

Generally I'm against having a in/out split. If something is generally useful I'm inclined to put it in, and if not leave it out. But I haven't thought about this much. What is the benefit of something being designated as "ouside the core" but still in the same git repo?

tl;dr: Quality of life for library users.

Different stability guarantees to the library users. Imgui have this. Code in contrib/ (glue code between imgui and glfw/whatever) is supposed to be copied into other projects, while the core imgui library is supposed to be linked in. The files copied into individual projects can be tweaked more easily.

And there is the difference in code style. The contrib part should be easy to tweak. The core part can be whatever. The counter example to readability would be the type magic I proposed awhile back.

---

About the usefulness of this feature, if it's useful only when the user understands the type system well, if the user can't understand the type system of Zig, it's confusing.

[VisenDev]

I think we pretty much figured out how we want to handle this