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

AP_Scripting: allow to mark arguments for no range check, optimize for size #18221

Merged
merged 5 commits into from Oct 11, 2021

Conversation

IamPete1
Copy link
Member

@IamPete1 IamPete1 commented Aug 5, 2021

This saves quite abit of flash. Majority of the saving from Os and about 3k from skipping range checks on floats that are currently -FLT_MAX FLT_MAX. Removing the initialization saves just less than 200bytes. Removing the extra else saves nothing.

I don't see any other low hanging fruit without getting a bit more into the lua side of things. There are quite a few things that look like they might be a easy saving, but in fact it is so easy that the compiler is already doing it in any case.

We could save lots more if we decided to remove more range checks, in many cases the c++ function will check for us in any-case, sensor index's for example, but I think more than I have done here really ought to be looked at individually, there could well be some functions where ignoring the range checks would be bad.

@IamPete1
Copy link
Member Author

IamPete1 commented Aug 5, 2021

Interesting to note that the generated bindings are now 5489 lines of code.

@IamPete1
Copy link
Member Author

IamPete1 commented Aug 5, 2021

Binary           text               data         bss          total
---------------  -----------------  -----------  -----------  -----------------
ardusub          -15352 (-1.0195%)  0 (0.0000%)  0 (0.0000%)  -15352 (-0.8682%)
blimp            -15344 (-1.2430%)  0 (0.0000%)  0 (0.0000%)  -15344 (-1.0251%)
arducopter-heli  -17008 (-1.0229%)  0 (0.0000%)  0 (0.0000%)  -17008 (-0.8835%)
arducopter       -17008 (-1.0062%)  0 (0.0000%)  0 (0.0000%)  -17008 (-0.8710%)
ardurover        -15352 (-1.0179%)  0 (0.0000%)  0 (0.0000%)  -15352 (-0.8671%)
antennatracker   -15344 (-1.1778%)  0 (0.0000%)  0 (0.0000%)  -15344 (-0.9804%)
arduplane        -16424 (-0.9701%)  0 (0.0000%)  0 (0.0000%)  -16424 (-0.8400%)

Saves quite abit more flash here than it did localy..... I had it saving 6k,

Turns out just to be the new compiler, old compiler Os saves 3k, new compiler it saves 13k

@IamPete1
Copy link
Member Author

IamPete1 commented Aug 5, 2021

With this patch scripting on Durandal still costs 135k! The majority of that is the included lua source, roughly 40k is bindings and such on the AP side.

Copy link
Contributor

@khancyr khancyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we need the initilization as this is compiled with gcc and not g++?

@tridge
Copy link
Contributor

tridge commented Aug 9, 2021

I think this looks good, but I'd like @WickedShell to take a look

@IamPete1
Copy link
Member Author

rebased

@IamPete1
Copy link
Member Author

IamPete1 commented Oct 3, 2021

rebased to get past semaphore.

@tridge
Copy link
Contributor

tridge commented Oct 4, 2021

@WickedShell please have a look. If you can't look in the next week I think we should merge

@tridge tridge merged commit 0f8bcbf into ArduPilot:master Oct 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants