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

Inefficient Print:write(data,len) shows message if used (only in debug mode) #4537

Merged
merged 7 commits into from
Mar 22, 2018

Conversation

d-a-v
Copy link
Collaborator

@d-a-v d-a-v commented Mar 20, 2018

+fix HardwareSerial that used it

size_t n = 0;
while (size--) {
size_t ret = write(*buffer++);
if (ret == 0) {
if (ret <= 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

ret is declared of type size_t, which should be unsigned, which means it can't be negative.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes very true. Blinded with ::read, arduino's api should all get a major reshape for coherency.

@@ -196,23 +196,30 @@ void uart_stop_isr(uart_t* uart)
}


void uart_write_char(uart_t* uart, char c)
void uart_do_write_char(uart_t* uart, char c)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe declare this one as inline.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks. I actually don't understand why inline does save flash space here.
This means one while loop and 2 calls with 2 parameters each take 16 more flash bytes than two while loops.
I was thinking gcc -Os would optimize this itself [...] Well, it would have if I had not forgotten static. Then I'd prefer the static way.

@devyte devyte changed the title unefficient Print:write(data,len) shows message if used (only in debug mode) Inefficient Print:write(data,len) shows message if used (only in debug mode) Mar 21, 2018
@d-a-v
Copy link
Collaborator Author

d-a-v commented Mar 21, 2018

Last "remove duplicate code" commit spares ~100 bytes of flash.
I also gain ~1kbps @878kbps in a serial loop BW test (0.1% yeah :)

edit:
On the other hand, the same test runs @800kbps before this PR. This is 10%.
And the 100bytes above compensate exactly (with my sketch) what is lost with the first commits (measured in debug mode so it's a win without).

So finally

  • a warning for inefficient code in debug mode, not bigger binary
  • or a smaller binary
  • better uart latency

edit 2:
This also fixes a bug introduced in the uart-overrun PR when debug mode is enabled.

@d-a-v d-a-v merged commit f8f205d into esp8266:master Mar 22, 2018
@d-a-v d-a-v deleted the uartminor branch March 22, 2018 00:24
bryceschober pushed a commit to bryceschober/Arduino that referenced this pull request Apr 5, 2018
…ug mode) (esp8266#4537)

* inefficient Print::write(data,len) shows message if used (only in debug mode)
* make HardwareSerial's write(data,len) efficient
* HardwareSerial: remove duplicate tests, move trivial code from .cpp to .h

(cherry picked from commit f8f205d)
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

Successfully merging this pull request may close these issues.

2 participants