Size reductions #1783

ccoffing · 2024-01-06T04:47:18Z

This is a grab-bag of little optimizations that have accumulated. It's only a few bytes and few cycles saved here and there, but I'm tired of sitting on them, and combined they cut 4k off the 2.8M media.

I think most of these changes stand on their own merits. But a few like to ls were specifically sought out to bring the binary down to the next smaller filesystem block size.

ghaerr · 2024-01-06T17:53:56Z

Hello @ccoffing,

Nice work 4K byte savings on the distribution!! :)

I am guessing that some of the literal string splits were designed for use with string merging at link time? Also, pretty tricky work on using a bit vector for testing for the C0 control codes. Fast, though initially hard to understand until I got it. Thanks for leaving in the switch statement as documentation.

I found one line that might be in error, along with a couple other small comments posted separately for your comment.

Thank you!

ghaerr · 2024-01-06T17:55:05Z

elkscmd/ash/linenoise_elks.c

    /* Cursor to left edge */
-    strcpy(seq, "\r");
-    abAppend(&ab,seq,strlen(seq));
+    abAppend(&ab,"\r",2);


Shouldn't this be abAppend(...,1)?

ghaerr · 2024-01-06T17:57:10Z

elkscmd/cron/cron.c

                    printf("    ");
                    printtrig(&tabs[i].list[j].trigger);
-                    printf("%s", tabs[i].list[j].command);
+                    fputs(tabs[i].list[j].command, stdout);


Are you sure that replacing printf with fputs is actually smaller here? In the case where printf is already used, it seems this just drags in another function, rather than saving space as it would if printf could actually be replaced in full at link time. Same for a few other cases with printf->fputs substitution in this commit.

Right, changing back to printf saves 16 bytes on the binary. Done.

ghaerr · 2024-01-06T17:57:36Z

elkscmd/cron/crontab.c

        seteuid(getuid());
        do {
-            fprintf(stderr, "Do you want to retry the same edit? ");
+            fputs("Do you want to retry the same edit? ", stderr);


Same here as commented above.

Turns out that this doesn't matter (function and binary are same size either way) since something else is already pulling fputs in. In general, fputs should take fewer cycles when no formatting is required. But since I don't know how/why fputs is getting pulled in already (maybe it can be trimmed out...) I'll revert this for now.

ghaerr · 2024-01-06T17:59:57Z

elkscmd/tui/cons.c

-        if (r == LINES - 1)
-            printf("\r");
-        else printf("\n");
+        putc(r == LINES - 1 ? '\r' : '\n', stdout);


Similar to printf->fputs described above, doesn't this just drag in fputc, while also including printf? I would guess this results in large .o and final codefile ? (Same issue in tty-cp437.c).

I think my initial motivation here was to save a few CPU cycles by avoiding the unnecessary format string parsing in vfprintf (which, when multiplied across screen redraws, might actually matter).

I just did a test:

Reverting cons.c and tty-cp437.c to printf as before, and doing a clean build:

$ du -b target/bin/ 1337353 target/bin/

Clean build with cons.c and tty-cp437.c putc change as first presented here. Slightly trading space for time:

$ du -b target/bin/ 1337369 target/bin/

Then converting cons.c and tty-cp437.c to fputs/putc (no printf), and doing a clean build:

$ du -b target/bin/ 1337321 target/bin/

So it seems that avoiding printf where possible in tty code is a [tiny] win, whether the goal is to optimize for space or time.

Also unnecessary to clear errno when it is guaranteed to be set

- More precisely trim out DEBUG code - Combine similar strings - Use libc string functions not custom. Some would be omitted ifdef SYS5, but we don't define that for other reasons; just rely on __STDC__. - Reduce size of linenoise

Where possible, prefer setbuf over setvbuf (64 bytes smaller) or skip explicitly setting to fully buffered when it isn't necessary. It's redundant to fflush stdout,stderr before exit because stdio already registers its own atexit to fflush stdout and stderr.

ghaerr · 2024-01-07T03:36:24Z

After looking a bit more closely at cons.c and tty-cp437.c, I agree with your original finding that printf should be replaced by fputs, mainly because fputs is already used in both (as is printf/sprintf) so primarily a speed gain without pulling in another function. Also, fputs.c calls putc so using that costs nothing once fputs is used.

However - some executables are linked with $(TINYPRINTF) (elkscmd/lib/tiny_printf.c). In those cases, beware of the substitutions used above because tiny_printf.c only defines a smaller/fast vprintf but no fputs or fputc, so with those programs its more tricky to decrease their size. This could all be fixed by changing a few things in tiny_printf.c but care needs to be taken as it was written only for programs that call printf/sprintf but don't do anything else with stdio.

Thank you!

ccoffing · 2024-01-07T04:18:43Z

However - some executables are linked with $(TINYPRINTF) (elkscmd/lib/tiny_printf.c). In those cases, beware of the substitutions used above because tiny_printf.c only defines a smaller/fast vprintf but no fputs or fputc, so with those programs

So regarding TINYPRINTF, it's possible with my changes that some binaries went up in size and some went down (but on average a win). I'll keep that in mind. Thanks!

ghaerr · 2024-01-07T04:27:39Z

regarding TINYPRINTF, it's possible with my changes that some binaries went up in size and some went down

Technically possible, but I don't think any of the programs you changed link with TINYPRINTF. In general, only very small programs that would otherwise use write / errmsg etc but can't because they need sprintf or a printf format designator have been linked with TINYPRINTF. It was invented just for that purpose, see elkscmd/rm.c for a good example.

TINYPRINTF is definitely not compatible with anything that calls fopen, and no file I/O other than stdin/stdout.

ELKS formatting; no functional changes

0d802d3

ghaerr reviewed Jan 6, 2024

View reviewed changes

ccoffing added 12 commits January 6, 2024 16:21

Shrink size of crontab; saves ~100 bytes per bin

1e93126

Fix various dead store & unused warnings; saves a few bytes

90f9ad1

Also unnecessary to clear errno when it is guaranteed to be set

Reduce size of ash (144 bytes)

c17968d

- More precisely trim out DEBUG code - Combine similar strings - Use libc string functions not custom. Some would be omitted ifdef SYS5, but we don't define that for other reasons; just rely on __STDC__. - Reduce size of linenoise

Reduce needless string variations so linker can combine

87ae27f

Shrink bc: Combine exit paths which fixes warning

ed4a321

Shrink libc: Keep leap year in-sync to avoid excess recalc

f12f1a4

Shrink touch: Advancing ptr more efficient than repeated indexing

27f3a97

Shrink cut: globals take more code to reference

6938738

Shrink ls

23a7377

Shrink tty routines, maybe make some faster

d6ce2fa

do_exit should be wontreturn

8f0e516

ghaerr merged commit bf32a25 into ghaerr:master Jan 7, 2024

ccoffing deleted the size-reductions branch January 7, 2024 04:18

Size reductions #1783

Size reductions #1783

Uh oh!

Conversation

ccoffing commented Jan 6, 2024

Uh oh!

ghaerr commented Jan 6, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghaerr commented Jan 7, 2024

Uh oh!

ccoffing commented Jan 7, 2024

Uh oh!

ghaerr commented Jan 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants