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

Bare-metal profile #9800

Merged
merged 1 commit into from Mar 1, 2019

Conversation

Projects
None yet
8 participants
@theotherjimmy
Copy link
Contributor

commented Feb 21, 2019

Description

This pull request provides the "bare-metal profile" that allows for mbed OS
to be built "bare-metal" or without an rtos. Requirinng the bare-metal
library limits the code built into mbed OS to the hal, drivers, and
platform.

The following table is the difference when using bare metal with release profile
on a simplified blinky example.

Module .text .data .bss
[fill] 111(-33) 8(+4) 2327(+206)
[lib]/c.a 17815(+0) 2472(+0) 89(+0)
[lib]/gcc.a 3112(+0) 0(+0) 0(+0)
[lib]/misc 204(+0) 4(+0) 28(+0)
main.o 56(+0) 0(+0) 4(+0)
mbed-os/cmsis 1033(+0) 0(+0) 0(+0)
mbed-os/components 0(-56) 0(+0) 0(+0)
mbed-os/drivers 80(+18) 0(+0) 0(+0)
mbed-os/features 0(-42) 0(+0) 0(-184)
mbed-os/hal 1396(-68) 4(-4) 66(-64)
mbed-os/platform 1961(-550) 260(+0) 201(-56)
mbed-os/rtos 0(-6929) 0(-168) 0(-5973)
mbed-os/targets 5232(-788) 12(+0) 381(-1)
Subtotals 31000(-8448) 2760(-168) 3096(-6072)

Total Static RAM memory (data + bss): 5856(-6240) bytes
Total Flash memory (text + data): 33760(-8616) bytes

Further, build time is reduced from 25sec to 8sec on my desktop and
1m22s to 13s on my mid-2014 mac book pro.

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[x] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

?

Release Notes

Mbed OS may now be built without the rtos by creating an mbed_app.json with
the following contents:

{
    "requires": ["bare-metal"]
}

@theotherjimmy theotherjimmy changed the title [DO NOT MERGE] Bare-metal profile ### Description [DO NOT MERGE] Bare-metal profile Feb 21, 2019

@ciarmcom ciarmcom requested a review from ARMmbed/mbed-os-maintainers Feb 21, 2019

@ciarmcom

This comment has been minimized.

Copy link
Member

commented Feb 21, 2019

@theotherjimmy, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

@theotherjimmy theotherjimmy changed the title [DO NOT MERGE] Bare-metal profile Bare-metal profile Feb 21, 2019

@theotherjimmy

This comment has been minimized.

Copy link
Contributor Author

commented Feb 21, 2019

I filled out the required sections to my satisfaction. Removing do not merge.

@0xc0170
Copy link
Member

left a comment

Looks simple !

into mbed OS to the hal, drivers, and platform.

How HAL gets in? I see requires only for platform and drivers. I could not find anything in these 2 folders that would pull in HAL (.json file requires HAL) ?

We got build profiles in the tools, this is in the components? What else is coming there? Should Readme be there describing the profiles ?

@theotherjimmy

This comment has been minimized.

Copy link
Contributor Author

commented Feb 22, 2019

@0xc0170 in order:

How HAL gets in? I see requires only for platform and drivers. I could not find anything in these 2 folders that would pull in HAL (.json file requires HAL) ?

It has no mbed_lib.json, so it's pulled in with the same rules as the targets directory and the user's application: No lib = pulled in.

We got build profiles in the tools, this is in the components?

Suggest a better location if you prefer.

What else is coming there? Should Readme be there describing the profiles ?

Nothing else is planned. Perhaps a Readme would be fine.

@0xc0170 0xc0170 requested review from ARMmbed/mbed-os-maintainers and bulislaw Feb 25, 2019

@0xc0170 0xc0170 added the risk: G label Feb 25, 2019

@cmonr cmonr requested a review from 0xc0170 Feb 25, 2019

@bulislaw

This comment has been minimized.

Copy link
Member

commented Feb 26, 2019

This PR is at risk of missing 5.12 release as it's marked as "needs: review". Code freeze is coming! On Friday 1st. Please review the changes ASAP.

@0xc0170 @bridadan @SenRamakri

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Feb 27, 2019

CI started whilst reviews are completed

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Feb 27, 2019

CI started for now to see where the PR stands.
Re-reviews still needed.

Ok, maybe it's time for sleep. Duplicate job stopped.

@bulislaw
Copy link
Member

left a comment

Please don't use "components" directory - these aren't the droids you are looking for (I'm so regretting allowing it to be created in the first place). The initial idea was to keep "drivers" for external HW components there, but now it became a go to directory for everything. Could we put the build profiles under tools? As it's a part of the build system. Also we need design document, but I won't block the merging based on that. Also changes to the Handbook and maybe blog post (?) to introduce the concept @SenRamakri

@mbed-ci

This comment has been minimized.

Copy link

commented Feb 27, 2019

Test run: SUCCESS

Summary: 12 of 12 test jobs passed
Build number : 1
Build artifacts

@theotherjimmy

This comment has been minimized.

Copy link
Contributor Author

commented Feb 27, 2019

@bulislaw They can't go under tools, as no device-related code is under tools, and it's ignored, and it's data, not a tool.

@theotherjimmy

This comment has been minimized.

Copy link
Contributor Author

commented Feb 27, 2019

As it's a part of the build system

This is not part of the build system. It's data, and I it's designed that way so that we don't have to worry about breaking changes or needing to update the online compiler for changes to the "build profile" (I still don't like calling it that, as it's neither a build nor a profile)

@theotherjimmy

This comment has been minimized.

Copy link
Contributor Author

commented Feb 27, 2019

The initial idea was to keep "drivers" for external HW components there, but now it became a go to directory for everything.

Perhaps that intention needs to be documented.

@theotherjimmy

This comment has been minimized.

Copy link
Contributor Author

commented Feb 27, 2019

Also changes to the Handbook and maybe blog post (?) to introduce the concept

What concept?

@theotherjimmy

This comment has been minimized.

Copy link
Contributor Author

commented Feb 27, 2019

@bulislaw Please suggest another location (other than tools; this is not a tool, and tools is not scaned so it would not work) for this library.

@bulislaw

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

This 4 lines represent bigger concept, corresponding tools changes are more than 4 lines I'm sure. The idea of build profiles deserves a design doc (it may be it was introduced with other PR).

(I still don't like calling it that, as it's neither a build nor a profile)

Actually the name needs to be changed as build profiles are "debug", "devel" and "release". Build variants? Scan profiles?

This is not part of the build system. It's data, and I it's designed that way so that we don't have to worry about breaking changes or needing to update the online compiler for changes to the "build profile"

How about build profile (eg debug) definitions? Isn't that data as well? I still think tools directory is the only logical place. If we need to work around the online compiler, I'd rather stuff it in platform than components.

@theotherjimmy

This comment has been minimized.

Copy link
Contributor Author

commented Feb 27, 2019

@bulislaw

Build variants? Scan profiles?

Scan profiles was suggested during design. It's more of a library set than anything else. We can call them scan profiles; I'm not going to bikeshed further on this topic in this PR. Instead, I'm going to remove the directory from the path.

How about build profile (eg debug) definitions? Isn't that data as well?

Yes it is, and a user can add their own and pass them in. These may be in the tools dir, as they are not scanned. That's the big deal here: bare-meta is a library, and like all other libraries it must be scanned.

I still think tools directory is the only logical place.

The tools directory is not scanned and therefore cannot conntain a library.

I'd rather stuff it in platform than components.

I'll move it to platform and It'll be platform/bare_metal. Does that sound reasonable?

@theotherjimmy theotherjimmy force-pushed the theotherjimmy:bm-lib branch to d1de429 Feb 27, 2019

Changes addressed. Please re-review.

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Feb 27, 2019

CI started

@cmonr cmonr added needs: CI and removed needs: work labels Feb 27, 2019

@mbed-ci

This comment has been minimized.

Copy link

commented Feb 27, 2019

Test run: FAILED

Summary: 1 of 8 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-ARM
@cmonr

This comment has been minimized.

Copy link
Contributor

commented Feb 27, 2019

Info: A CI config issue appears to be affecting NUMAKER_PFM_M2351 builds. Please ignore build errors against the target for now.

Other build failures should still be investigated, if any. Will restart CI when appropriate.

@bulislaw
Copy link
Member

left a comment

Looks good for now, we may need to rethink that later (if we add more profiles, we'll need an uber directory in platform to keep it tidy).

@0xc0170

This comment has been minimized.

Copy link
Member

commented Feb 28, 2019

CI restarted

@theotherjimmy

This comment has been minimized.

Copy link
Contributor Author

commented Feb 28, 2019

@bulislaw Agreed, having this in an "uber-directory" is a bridge we can cross when we get there.

@mbed-ci

This comment has been minimized.

Copy link

commented Feb 28, 2019

Test run: FAILED

Summary: 1 of 13 test jobs failed
Build number : 3
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_exporter
@alekla01

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2019

Ci restarted (iar8 exporter issue will be resolved separately).

@loverdeg-ep

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2019

@theotherjimmy It's probably obvious from your description but does this library/profile leave out eventQueues?

Maybe eventQueues are considered to be in rtos territory, but no matter how simple the application I'd never go bare metal without an event loop.

@cmonr cmonr added ready for merge and removed needs: CI labels Feb 28, 2019

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2019

@loverdeg-ep You bring up an interesting point. Since we'd like to bring this in before code freeze, I'd propose opening an issue for the question, linking to this PR.

I personally know of many simple use-cases where an RTOS is overkill, and even an Event Queue might not be needed.

@cmonr cmonr merged commit eff8b1d into ARMmbed:master Mar 1, 2019

30 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/build-ARM Success
Details
jenkins-ci/build-ARMC5 Success
Details
jenkins-ci/build-ARMC6 Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR8 Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage RTOS ROM(+0 bytes) RAM(+0 bytes)
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARM Success
Details
jenkins-ci/mbed2-build-ARMC5 Success
Details
jenkins-ci/mbed2-build-ARMC6 Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR8 Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Local astyle testing has passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/doxy-spellcheck Local doxy-spellcheck testing has passed
Details
travis-ci/events Passed, runtime is 10274 cycles
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/include_check Local include_check testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8408B (+0.00%)
Details
travis-ci/psa-autogen Local psa-autogen testing has passed
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
travis-ci/tools-py3.5 Local tools-py3.5 testing has passed
Details
travis-ci/tools-py3.6 Local tools-py3.6 testing has passed
Details
travis-ci/tools-py3.7 Local tools-py3.7 testing has passed
Details

@cmonr cmonr removed the ready for merge label Mar 1, 2019

@loverdeg-ep

This comment has been minimized.

Copy link
Contributor

commented Mar 1, 2019

@cmonr Not trying to stop the press, just vacuuming info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.