Skip to content

Conversation

@apocelipes
Copy link
Contributor

@apocelipes apocelipes commented Oct 13, 2024

Optimze:

  • reduce procFilePath buf size: On SunOS change the size from PATH_MAX to 128.
  • Only allocate the actual size of exePath, do not allocate a PATH_MAX length memory.

Why I did the last two optimizations:

Common values ​​for PATH_MAX on Linux are 4096 or 8192, but the common exePath for example "/home/apocelipes/.local/bin/flashfetch" is only 38 bytes and "/usr/bin/bash" is only 13 bytes. If we allocate exePath with PATH_MAX size, it would waste almost 4k heap memory. This optimization waste one memcpy but the path is short in the most common cases and we no longer need to allocate a huge block of heap memory, so there's no noticeable performance regression.

The 4k stack buf is also a big space cost, fortunately we have changed all ffProcessGetInfoLinux's callers from recursion to looping early this year, so the cost is one-time and there is no worry about the stack overflow.

SunOS's PATH_MAX is commonly 1024, the waste is much smaller than on Linux but still wastes nearly 1k memory. That's why I did the same optimizations.

Memory usage before the optimization:
before

after:
after

@apocelipes apocelipes changed the title Processing(Linux, SunOS): simplify the code and little optimizations Processing(Linux, SunOS): little optimizations Oct 14, 2024

#ifdef __linux__

char filePath[64];
Copy link
Member

Choose a reason for hiding this comment

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

You won't save memory by shrinking stack variables. There is no difference as long as the stack doesn't overflow.

Stack is statically allocated when the thread is created. It's always there, no matter if you use it or not.

Copy link
Member

Choose a reason for hiding this comment

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

To be honest I'm not a fun of calculating how many bytes a number string will take. What if you make a mistake? A reasonable safe number is better for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

char procFilePath[PATH_MAX] is an obvious waste. I will keep this change and discard others.

Copy link
Contributor Author

@apocelipes apocelipes Oct 14, 2024

Choose a reason for hiding this comment

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

Stack is statically allocated when the thread is created. It's always there, no matter if you use it or not.

The real world thing is that linux kernel only allocates a small fixed size of stack and grows it when needed (only the main thread, other threads such as are created by pthread will get a fixed stack).
image

But I agree that save 100 bytes does not make sense here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest I'm not a fun of calculating how many bytes a number string will take. What if you make a mistake? A reasonable safe number is better for me.

128 on SunOS is safe and you have already used it, it's better than PATH_MAX. I wont change other buf size.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Really? That is interesting. AFAIK a stack memory must be continuous, which means that you must move all used memory to the new memory space without modifying stack pointers (rbp rsp etc). I wonder how it can be done.

By the way, stack address, frame pointer and other things are usually virtual memory, keep it be continuous is easy because the kernel can modify the page table and make discontiguous physical memory looks like continuous. And this is no so much harm for the cache.

Copy link
Member

Choose a reason for hiding this comment

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

I dont think so. It is easy ONLY if you have control to the physical memory address. I dont think it is possible for a userland program.

Copy link
Member

Choose a reason for hiding this comment

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

If it is possible, why do we have to move memories when appending a string or array?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I dont think so. It is easy ONLY if you have control to the physical memory address. I dont think it is possible for a userland program.

Stack is not controled by user space, when you add the stack pointer and access the stack memory, if it did not allocated a physical memory area, a page fault will be caused. The kernel will handle the PF and allocate physical memory, finally add it to the page table.
For detail you can see this: https://stackoverflow.com/questions/46790666/how-is-stack-memory-allocated-when-using-push-or-sub-x86-instructions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it is possible, why do we have to move memories when appending a string or array?

Why we should move these memory, because we can not control the virtual memory address on the user space. The Linux kernel only handles few special cases for us. A normal mmap used by malloc wont care about the virtual memory address. Even in the kernel only a few parts can access to physical memory and arbitrarily set the virtual memory address.
(brk/sbrk can make the small heap allocations be continuous, but they are syscalls, not on the user land).

exePath->length = (uint32_t) length;
buf[length] = '\0';
ffStrbufEnsureFixedLengthFree(exePath, (uint32_t)length + 1); // +1 for the NUL
ffStrbufAppendNS(exePath, (uint32_t)length, buf);
Copy link
Member

Choose a reason for hiding this comment

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

This does save some heap memory. Make sense.

@CarterLi CarterLi merged commit bbc9cf4 into fastfetch-cli:dev Oct 14, 2024
15 checks passed
@apocelipes apocelipes deleted the process-openat branch October 14, 2024 03:40
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