arbitrary op deletion

[catfact]

made a function in net.c : net_remove_op()

this is supposed to delete the selected operator and perform all necessary re-indexing. probably doesn't work though.

[ghost]

I'm going to try & tackle this later on. Seems like the pain to users of merging #260 would be greatly eased if they could delete old versions of grid/life in their scenes, then rebuild only those parts according to new grid/life, without unpicking the whole thing...

[ghost]

OK so interesting first read/dip-through the code I see that arbitrary op deletion seems to be at least somewhat functional. Just set up a quick test whereby menu delete always erases the first op added, rather than the last. E.g adding 10 identical ops, then deleting from first->last and they all display alright on all menus but attempt to change input params & the input stays fixed at 0. Guess this means the plumbing to reconnect input functions is getting mangled. Should be simple enough to figure out what's going on, as it doesn't outright crash. Saw intermittent hard crash with mixed-operator netlist, which leads to a theory mis-indexing the operator to be removed somehow.

Gonna hold back again on emulator until reading a lot of code and understand every line in net_remove_op, then litter all relevant source files with muchos debug and reflash...

[ghost]

Tantalisingly close now but this:

https://github.com/rick-monster/aleph/commit/ed830128b03882dc8b38426c90bda2613af01c58

is not quite right - pretty sure once that's fixed we're good to go. Anyone see where I'm going wrong?

EDIT:
still not working with current HEAD gonna have to dig deep maybe emulation is the only way to figure out this byte-bashing malarky.

UPDATE: OK - seem to be able to ping the network interactively inside beekeep_gtk on my laptop using gdb wish I'd realised this was already possibility! Somehow didn't quite put 2 and 2 together...

still not quite seeing what's up with the plumbing but maybe stand a fighting chance now.

[ghost]

The following ops blow up when you delete them with the arbitrary op deletion code:

• screen
• bars
• bars8
• bignum
• mout_cc
• mout_note
• serial

Notice all these ops use interrupts - maybe pointers to the moved memory get copied round all over the place & driven from ISR? The more I think about this the more I wonder why we're not just using alloc & free to manage all the memory for bees ops?

[catfact]

maybe pointers to the moved memory get copied round all over the place & driven from ISR?

ah damn, i think that's exactly the case. a midi operator has its address pushed to a list that is looped over on midi handler. a polled operator adds a soft timer with its address as the callback argument. and so on.

arg....

The more I think about this the more I wonder why we're not just using alloc & free to manage all the memory for bees ops?

the reason is actually pretty basic. bees was written before we had heap allocation working!

[ghost]

OK well the silver lining is I learnt how to debug bees on linux (beekeep-gtk, gdb). After this weekend, think my understanding of bees is now extensive enough to bite the bullet & run a big overhaul implementing:

• outs-to-play-screen (see below)
• allocate-operators-from-heap
• re-jig the underlying data-structure for bees netlist connections so we don't repeat information & get continually mangled on resultant accidental complexity.

In order to achieve this bees bangs should become:

typedef struct {
u8 opIdx;
u8 opInIdx;
} beesIn;

typedef struct {
u8 opIdx;
u8 opOutIdx;
} beesOut;

net_activate(beesIn targetAddr, s16 value, beesOut sourceAddr)

In this scheme we can totally get rid of net->ins & net->outs because data therein can be derived from net->ops. We then use an accessor to display a flat list of all inputs/outputs. e.g instead of directly accessing net->ins[globInIdx] you calculate the {op,opInIdx} address each time by seeking through the list of ops & counting their inputs:

beesIn get_beesIn_at(u16 globInIdx);

Can add some caching to that function if necessary...

The obvious ugly-duplication/insanity in new scheme would be that each op must also know it's own index in the global ops array in order to enable the outs-to-play-screen feature. I don't see a way to avoid that - should probably chew on it a bit & try to hammer out that bump before starting a big refactoring effort...

[ghost]

Ummm... @catfact will this also entail rewriting scene pickling/unpickling!? Too late to turn back I've started already. Looks like things are about to get a little hairy...

[catfact]

it seems fine to me as long as it doesn't impose too much extra work during the execution of net_activate() (or whatever)

will this also entail rewriting scene pickling/unpickling!?

i guess so, if you want to get rid of the ins/outs list altogether.

if you're considering an overhaul of this scale, then there might be other things to change:

• getting rid of NET_INS_MAX and so on
• making presets dynamically allocated as well

the point being to make scene storage much much smaller, and actually reflective of the complexity of a scene. that way simple scenes could be stored in internal flash instead of on sdcard.

i don't know what kind of problems we might run into with heap fragmentation (or not)...

[ghost]

ok I just ploughed ahead. nearly halfway through rewriting net.c...

net has become solely an array of operators not a struct/class thingy, NET_INS_MAX is gone. Only barely scratched the surface of the pickling/unpickling malarky - have to admit I still don't really understand how scene file format works, though I did notice every bees scene is the exact same file size so I guess it's more or less a giant memory-dump at present?

Re: heap fragmentation if it becomes a problem having malloc/alloc do the heavy lifting we can try pre-allocating a huge block as before, but each successive op regularly spaced according to the size of the largest operator. Then reclaiming memory is super easy even a linked list could be a cute way to manage a pool of uniform-size blocks. How many ops would fit in RAM with that scheme? Don't know off hand the biggest operator or memory limit...

[catfact]

every bees scene is the exact same file size so I guess it's more or less a giant memory-dump at present?

it's not exactly a memory dump, the point of all the pickling weirdness is that things like each operator class, and each input value, are represented in a predictable way (including forcing endianness.)

i can't remember why i forced all the scenes to be the same size by writing the full complement of inputs, presets, etc. waaay more stuff than is really needed. i can't remember why this was necessary, it was some desperate crash-avoidance gambit.

actually i do remember that part of the problem was this very stupid situation where DSP inputs are in the same list as operator inputs, which we really should fix as part of this overhaul too, i guess... (#87)

each successive op regularly spaced according to the size of the largest operator. Then reclaiming memory is super easy even a linked list could be a cute way to manage a pool of uniform-size blocks.

that seems smart. i wonder if it would be too much of a brainf*k to make them irregularly spaced though, and use list elements that include the block sizes. because it seems to me that the size of the largest operator is quite a bit bigger than the size of the smallest.

the biggest op would probably be op_list16 or something that just has lots of ins and outs.
ohhh wait... i take that back, big time. the screen operator has a full graphics buffer, so that has got to be the biggest by far... as soon as i'm on the proper machine i'll try spitting out a full list.

the big 'grid app' operators store all their state in static vars(!) which means the actual op objects are quite small and will break spectacularly if you try and use more than one of each.

[ghost]

Ok so first thing could be get the thing back up & running with malloc/free, suck it & see. Also my scheme is to represent the DSP as an fake-operator located at address opIdx 254. Address 255 is No Connection.

Looking forward well it's an interesting crossroad. Just thought hard about this & the only way to get a heap-compaction scheme as originally proposed AND let callbacks peek into that memory in a safe way is to hold a global un-ordered array of pointers to the dynamic op-heap, which must be locked during heap-compaction, then updated after. Then those pointers themselves must be managed using the fixed-block-size scheme (both the op & callback have a pointer to the pointer). That way when the opIdx & op Base-address changes due to heap-compaction, the callback can find it's way to the new location in op-memory. op_t *net[256] becomes:
opIdx_t net_idx[256];
op_t *net_unordered[256];
bool net_unordered_slots_empty[256];

This seems crazy complex to me!

Instead how about 2 mempools - bigOp & smallOp. smallOp[256] is up to a maximum of, say 128 bytes (that makes 32kB total for smallOps). Then bigOp[16] holds 16 objects each up to maximum of 16kB (256kB total). Both these regions get one-time allocated when bees is initialised:

#define N_BIGOPS
#define N_SMALLOPS
typedef u8[128] smallOp_t;
typedef u8[1024 * 16] bigOp_t;
smallOp_t *smallOpPool;
bigOp_t *bigOpPool;
smallOpPool = malloc(sizeof(smallOp_t) * N_SMALLOPS);
bigOpPool = malloc(sizeof(bigOp_t) * N_BIGOPS);
op_t *net[256];

Then maintain a linked list of empty memory slots for both bigOpPool & smallOpPool, which can be pushed & popped.

we have a whole megabyte of SRAM, right? So I think this scheme easily fits within the available memory and allows to host some pretty intense bees patches. Do those numbers sound about right?

[ghost]

So I have fixed arbitrary op deletion for all the 0-output operators (subtle mistake here) and for all of the screen-drawing ops:
op_bars
op_bars8
op_bignum
op_screen

However there are 3 more operators using polled-callbacks:

op_metro
op_delay
op_ww

The callback data struct must always point to somewhere allocated with malloc. So one way to hack it is define a new struct for those operators duplicating the data required by the callbacks. The operator logic must then be responsible for keeping the duplicated data in sync. However that is impossible for op_metro & op_delay because they need to know the operator outputs, which are not controlled from within the operator logic, but in a generic way.

I think now that the only way to make this work may be to add one more hack to the arbitrary-op-deletion code in net.c . That is net_remove_op(opIdx) must also interrogate the event queue, and correct any pointers held within kEventAppCustom events pointing to memory which has just been reshuffled.

Ugly I know but it may be the only way!
EDIT: so there's an additional task, which is to adjust memory locations of timers that have moved. Maybe I can hack this by checking for those 2 ops, then disabling/renabling the timer if it's already active - also update the position of metro->op_poll->op for these 2, as it has now moved. urggghhhh this is so horrible!

[ghost]

ok no I just tried getting rid of op_pool altogether & allocating all ops using malloc/free. It seems to work without problems. Now I will try to grab the arbitrary op deletion function from the other branch and make that work the same. Then we have no problems other than the threat of memory fragmentation. We can mitiagate the effects of memory fragmentation by setting a minimum size for the call to op malloc - every operator has to sit in at least 128 bytes, so their memory can always be reclaimed for another op that fits in the 128 bytes.

If it nonetheless becomes a problem then I will implement the dual memory pool idea hammered out earlier in this thread (bigOp/smallOp linked-list mempool)...

[ghost]

Sorry my previous post documenting bugs with new malloc/free bees appears to be absolute nonsense so deleted it off this trhead. I can't find or reproduce any bugs now, whatsoever!

Hurrah! Yann_Coppier on the forum seems to do a very thorough job of testing I'm going to try and enlist his support again...

[ghost]

woohoo!