Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change stack size based on app #181

Closed
alistair23 opened this issue May 15, 2020 · 16 comments
Closed

Change stack size based on app #181

alistair23 opened this issue May 15, 2020 · 16 comments

Comments

@alistair23
Copy link
Collaborator

Currently we set the stack size per board. The problem with this is that stack usage changes per app and not per board.

For example the blink example can run with 1024 bytes of stack (and probably even less) while more complex examples such as the hmac need a larger stack size. Setting a standard size per board means we either waste space in the case of the blink example or we can't fit the hmac example.

By setting a stack size per app we can fit smaller apps on smaller boards (such as the HiFive1) and then generate an error if a user builds a larger app. The error will come when a user tries to load this larger app on the board as it won't fit.

For a user example:

  • A user builds a blink example for HiFive1 where the app sets a 1024 byte stack
    • This example fits on the board and runs
  • A user builds the hmac example for HiFive1 with a 2048 stack and tries to load it
    • This fails to load as the binary is too big

The user now knows that the hmac example won't fit on their board, while the blink example will. Otherwise with a board specific stack size the user can generate a hmac example and then has to debug why it doesn't run.

@rajivr
Copy link
Contributor

rajivr commented May 15, 2020

FWIW, PR #180 is a good change and something that libtock-rs should consider. While I'm not a downstream user of libtock-rs, I plan on adopting this change into our tree.

Another thing to note it that it is possible for elf2tab to automatically calculate the stack size. I use a program called elf2tbf with custom TBF headers, and calculate the stack size directly based on ELF section size.

https://gist.github.com/rajivr/8f773c7d057c19393bffc0f0354bc6ec

@torfmaster
Copy link
Collaborator

I would say that the top priority should be to provide users of the library a way to change the stack size somehow in an comprehensible way (using a macro, an environment variable, whatever). As examples are somehow special (many things cannot be configured individually as feature flags, linker scripts) I wouldn't put too much effort in making the stack size in each example configurable, I'd rather find another way.

As time permits I'll prepare a solution more ergonomic than @alistair23 's solution.

@torfmaster
Copy link
Collaborator

I currently see no value of mixing code and linking more than necessary. I will prepare a PR using the handlebars template engine to create the linker file at compile time and filling the stack size with a value from a environment variable.

@jrvanwhy
Copy link
Collaborator

jrvanwhy commented May 20, 2020

I will prepare a PR using the handlebars template engine

Please avoid using handlebars, as it is avoidable and too large to review (see our third-party dependency policy).

@Woyten
Copy link
Contributor

Woyten commented May 21, 2020

Please avoid using handlebars, as it is avoidable and too large to review (see our third-party dependency policy).

Makes kind of sense although I am not totally convinced that the third-party dependency policies should be applied to build dependencies.

@torfmaster Would it possible to write our template "engine" ourselves using, for example, string.replace("{{FOO}}","foo"?

@alistair23
Copy link
Collaborator Author

Just be careful, I still don't see how each app could set that then. Also I'm not sure that is any clearer as a user auto-generating a linker file seems very confusing and hard to debug.

@torfmaster
Copy link
Collaborator

Also I'm not sure that is any clearer as a user auto-generating a linker file seems very confusing and hard to debug

I think this is the way to go latest for multi-app support.

I still don't see how each app could set that then.

For a consumer of our library setting the stack size then works by setting an environment variable. I think in "each app" you refer to the examples in this crate: I would consider this use case as artificial as a users of the library will most probably only have one application in their crate.

@torfmaster
Copy link
Collaborator

I totally agree that handlebars-rust (which is not too large) is avoidable for now.

Please avoid using handlebars, as it is avoidable and too large to review (see our third-party dependency policy).

I maybe don't understand this sentence here: "This policy does not currently apply to host-side tools." Doesn't this imply that build-dependencies are excluded from the policy for now? In any case I think the policy should differentiate between different kinds of dependencies (compile, dev-dependencies, linked depdendencies...).

@torfmaster
Copy link
Collaborator

@alistair23 Maybe another solution would be also suitable. What about putting each example file into a different crate outside the libtock-rs folder.

Pros:

  • we could set the stack/heap/kernel heap size per app using cargo aliases/Makefiles
  • configuring features per app is much more straightforward
  • we could put a bit of documentation (README.md) explaining the example a bit
  • we wouldn't have to struggle with the limited flexibility of cargo examples anymore
  • we could evaluate how suitable libtock-rs is for publishing to crates.io with respect to the questions
    • How well does our build tooling work when used as external dependencies?
    • How flexible can an app be configured (feature flags, exchange linker scripts ...)?
    • Is using our library straightforward or can the usability be improved?

Cons:

  • maybe this is not a con but we will have to re-organize the folder structure and the resulting workspace will contain a lot of crates

Your PR raised a lot of interesting questions, let's try to answer them.

@alistair23
Copy link
Collaborator Author

Splitting the examples into separate crates seems fine. I'm guess that means you need to navigate around the directories to build them, which seems like a pain, but the benefits will probably outweigh that.

@torfmaster
Copy link
Collaborator

Splitting the examples into separate crates seems fine. I'm guess that means you need to navigate around the directories to build them, which seems like a pain, but the benefits will probably outweigh that.

Would it be okay to go with the changes of the PR and split the examples later? I fear that this will involve moving of directories and changing/splitting of Makefiles, travis config etc. making this a bigger PR.

@alistair23
Copy link
Collaborator Author

I'm not clear what you mean?

@torfmaster
Copy link
Collaborator

I'm not clear what you mean?

I referred to merging the changes included in #184 and splitting examples (i.e. enabling configuration per-app) in another PR.

@alistair23
Copy link
Collaborator Author

#184 isn't related to this issue though is it?

@torfmaster
Copy link
Collaborator

Combing #184 + the splitting the examples into folders would solve this issue, I guess.

@alistair23
Copy link
Collaborator Author

Closing as this was addressed by: #244

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants