Increase limits of state arrays

[burnsauce]

Some of the data structures could really benefit from dynamic allocation of memory to permit more flexible scripts.

An example of this is the exec stack. The are 8 frames that are statically allocated, and then no more. Dynamic allocation could allow a user much more potential for recursion.

Delays are another example, as are the queue and stack.

Note that extension to the execution stack and delays would need to be balanced by a scheduler to prevent busy scripts from causing instability.

Is there any objection to pursuing this sort of change?

I'm thinking of using dope vectors with chunk allocation that adapts to upward pressure on these resources. Features that don't get used won't take up memory.

[samdoshi]

So firstly, there is a lot of free RAM available. Can you not just increase the size of all the arrays (i.e. stay with the static allocation)? Or will that not be enough stuff?

Otherwise, so long as:

• you can prove that static allocation won't be enough (i.e. show me that the extra code burden is worth it)
• you're using a bespoke allocator, rather than malloc (I think malloc will end up giving us heap fragmentation issues, right?)
• the allocator is well abstracted (so that those contributors that are less skilled can still add OPs, etc)
• you code defensively for out of memory issues (as thou shalt not crash thy Teletype).

Then I'm cool with it.

I would suggest that such a change should be done in isolation from any other code changes.

I will also suggest that you consider the cost-benefit of such a change. The amount of time required to make it may be better spent elsewhere. (e.g. updating the file import and export so that we can switch to having 8 patterns instead of 4).

[tehn]

highly agree with sam on this.

we can selectively raise the statically allocated elements for the next release.

i'd prefer to postpone this sort of major surgery until we get a few things wrapped up that are already in line:

• screen glitching
• 2.1 features
• 2.2 features (scanner's grid ops)
• usb saving
• timeline

but of course if you'd like to work on this as an isolated branch, feel free!

On Tue, Sep 19, 2017 at 4:44 AM, Sam Doshi notifications@github.com wrote:

So firstly, there is a lot of free RAM available. Can you not just increase the size of all the arrays (i.e. stay with the static allocation)? Or will that not be enough stuff?

Otherwise, so long as:

• you can prove that static allocation won't be enough (i.e. show me that the extra code burden is worth it)
• you're using a bespoke allocator, rather than malloc (I think malloc will end up giving us heap fragmentation issues, right?)
• the allocator is well abstracted (so that those contributors that are less skilled can still add OPs, etc)
• you code defensively for out of memory issues (as thou shalt not crash thy Teletype).

Then I'm cool with it.

I would suggest that such a change should be done in isolation from any other code changes.

I will also suggest that you consider the cost-benefit of such a change. The amount of time required to make it may be better spent elsewhere. (e.g. updating the file import and export so that we can switch to having 8 patterns instead of 4).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/monome/teletype/issues/101#issuecomment-330471899, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPEcKaUIqM_71S646qbLxNNRtMxKy_Sks5sj38GgaJpZM4Pb5Js .

[scanner-darkly]

one thing to consider is maintainability. i like the idea of moving towards a more mature architecture in general, but this kind of implementation can introduce hard to track bugs. so then the question is, is a payoff big enough to justify this? (and i tend to think that in general simple is beautiful - i work in software industry and i think there is often a tendency to overengineer something without considering the ratio of the benefit it provides vs the increased cost of maintenance)

[burnsauce]

I'm fine with incrementing static limits if RAM is not a real concern. I'm going to suggest a few here:

• EXEC_DEPTH from 8 -> 256 or more (if while limit is okay at 10,000 or even 1,000, then this should be no concern from a CPU standpoint.)
• STACK_SIZE from 8 -> 256 or more (to support meaningful recursion along with EXEC_DEPTH)
• COMMAND_MAX_LENGTH 12 -> 16? I run out of these with all the short aliases
• STACK_OP_SIZE from 8 -> COMMAND_MAX_LENGTH (if the editor can use that many, why not the stack?)
• Q_LENGTH 8 -> 64 (to match the length of a pattern)

[tehn]

i'd suggest doing some testing with realworld uses of a very large EXEC_DEPTH and STACK_SIZE. for example, if you're doing something that recurses 256 times, do the timers stay on time? do trigger inputs get compromised? does the screen go totally nuts?

COMMAND_MAX_LENGTH should certainly be increased for max line with short aliases. STACK_OP_SIZE should match, yes. that seems like a bug. Q_LENGTH, yes i think this is a good increase.

[samdoshi]

A large EXEC_DEPTH will have you run into stack overflow issues. SCRIPT recurses via run_script_with_exec_state. You'll need to trampoline it (non-trivial), or prove to me that the stack won't blow (also, not trivial without a JTAG).

STACK_SIZE controls the size of the stack to execute a single command. As it stands I believe it is impossible for it to be larger than 8 (after all, no OP takes 8 inputs from the stack). If it's needed (say if my suggested array OPs ever get merged), then it could be increased. But too large values just end up masking underlying bugs.

COMMAND_MAX_LENGTH, I end up increasing this on my own builds all the time. For the same reason, lots of subcommands and aliases on a single line. But, this value also controls how much space a scene takes up in ROM. An increase from 12 to 16 will lead to a non-trivial increase in the amount of flash ROM required to save the 32 scenes. Personally I am in favour of it, but we need to ask if that space is need for other things (i.e. more timeline entries per scene).

I'd be very happy with Q, S and DEL being allowed a lot more entries each though.

[tehn]

timeline will need a considerable amount of space, so we're on track already to reduce the number of SCENEs. perhaps even in half, down to 16. i'm personally fine with this-- but this change should not be introduced until there is a more user-friendly USB read/write UI, so that individual scenes can be loaded up and moved around so the 16 doesn't seem like such a tragic compromise.

On Wed, Sep 20, 2017 at 11:40 AM, Sam Doshi notifications@github.com wrote:

A large EXEC_DEPTH will have you run into stack overflow issues. SCRIPT recurses via run_script_with_exec_state. You'll need to trampoline it (non-trivial), or prove to me that the stack won't blow (also, not trivial without a JTAG).

STACK_SIZE controls the size of the stack to execute a single command. As it stands I believe it is impossible for it to be larger than 8 (after all, no OP takes 8 inputs from the stack). If it's needed (say if my suggested array OPs ever get merged), then it could be increased. But too large values just end up masking underlying bugs.

COMMAND_MAX_LENGTH, I end up increasing this on my own builds all the time. For the same reason, lots of subcommands and aliases on a single line. But, this value also controls how much space a scene takes up in ROM. An increase from 12 to 16 will lead to a non-trivial increase in the amount of flash ROM required to save the 32 scenes. Personally I am in favour of it, but we need to ask if that space is need for other things (i.e. more timeline entries per scene).

I'd be very happy with Q, S and DEL being allowed a lot more entries each though.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/monome/teletype/issues/101#issuecomment-330892332, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPEcBc-2asZBz2fZZMCK5IHcP8C1irVks5skTHJgaJpZM4Pb5Js .

[scanner-darkly]

i had to increase STACK_SIZE to 10 for some of the grid ops (i'm not really comfortable with such a large number of parameters but this was the result of redesigning the ops several times and seemed like the best compromise, and this helped to reduce the overall number of grid ops).

[burnsauce]

A large EXEC_DEPTH will have you run into stack overflow issues. SCRIPT recurses via run_script_with_exec_state.

I'll see what I can do about a trampoline in a branch.

[samdoshi]

I'll see what I can do about a trampoline in a branch.

I think such a change is best done in isolation from anything else. If it was me I would hold off doing any work on it until the other changes have landed and have settled. I would recommend you do the same.

[burnsauce]

Am I good to bump COMMAND_MAX_LENGTH to 16 in 2.1 despite the flash size concern? Sounds like 2.2 will need it anyways. I haven't done a static analysis of the flash ram so if anyone has those numbers handy or can adequately assuage any such concern, I will comfortably implement the change.

STACK_OP_SIZE and Q_LENGTH are all RAM, so I can incorporate those changes without concern.

edit: I'm tackling a USB filesystem overhaul for the next available release after it's done so that we can move on to TL.

[tehn]

i think yes, it's good to change this now.

we can address flash usage when timeline happens.

On Tue, Sep 26, 2017 at 8:35 PM, Poindexter Frink notifications@github.com wrote:

Am I good to bump COMMAND_MAX_LENGTH to 16 in 2.1 despite the flash size concern? Sounds like 2.2 will need it anyways. I haven't done a static analysis of the flash ram so if anyone has those numbers handy or can adequately assuage any such concern, I will comfortably implement the change.

STACK_OP_SIZE and Q_LENGTH are all RAM, so I can incorporate those changes without concern.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/monome/teletype/issues/101#issuecomment-332374605, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPEcOKMNs2FS-k6sabN6XtEuoMIjONdks5smZhLgaJpZM4Pb5Js .

[samdoshi]

If you do end up needing more flash, the best way would be to reduce the number of lines for the scene description text. Currently it's 32, it could easily be reduced to 16. You'll just need to make sure that the import code handles the shorter size gracefully.

[burnsauce]

This issue can be closed with the release of 2.1.0 IMO.