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

Add support for Fast FAP loading #89

Closed
str4d opened this issue Jun 21, 2023 · 7 comments · Fixed by #99
Closed

Add support for Fast FAP loading #89

str4d opened this issue Jun 21, 2023 · 7 comments · Fixed by #99
Labels
enhancement New feature or request

Comments

@str4d
Copy link
Contributor

str4d commented Jun 21, 2023

flipperdevices/flipperzero-firmware#2790 bumps the SDK to 31 with 15x faster loading of FAP files. It requires adding .fast.rel sections, so we will need to alter flipperzero-rt to enable it.

@dcoles
Copy link
Collaborator

dcoles commented Jun 22, 2023

Looking at the PR, I think what it's doing is creating a hash table for symbol lookup, rather than a linear search.

This might need to be done as some sort of post-processing step as I don't think it's possible to do this in build.rs or the linker script. This appears to be what scripts/fastfap.py is for.

@DrZlo13
Copy link

DrZlo13 commented Jun 22, 2023

Yep, fastfap.py is post-processing elf files not only for hashing (also, this does not affect the speed, it is size optimization), but also to create an optimized relocation structure for our elf loader. This removes a large number of calls to the storage, and that's how such an acceleration is achieved.

Instead of "Each offset in .rel refers to .sym, then .sym refers to .symstr, and then we get a hash for the string from .symstr and then we apply this single relocation" we now have ".fast.rel, which contains a hash of the function name and offset, where this relocation needs to be applied”.

I recommend you to wait until this feature is released because the format of these sections is subject to change. I also left a fallback to the old usual linking process if optimized sections is not present.

@dcoles
Copy link
Collaborator

dcoles commented Jun 22, 2023

@DrZlo13 Thanks for the information. Definitely appreciate the fallback, so this isn't an urgent thing to address.

@dcoles dcoles added the enhancement New feature or request label Jun 22, 2023
@str4d
Copy link
Contributor Author

str4d commented Aug 9, 2023

This was released in firmware version 0.86.1, and once #93 merges we will require at least 0.87.0, so we can start working on this.

@str4d
Copy link
Contributor Author

str4d commented Aug 26, 2023

This is now implemented in #99. I think I got the encodings right, and it appears to work (cargo test gets noticeably faster) but I'm unsure if the MD5 for tempfile names in fastfap.py is just for convenience or meant to be load-bearing.

str4d added a commit to str4d/flipperzero that referenced this issue Aug 26, 2023
@DrZlo13
Copy link

DrZlo13 commented Aug 27, 2023

@str4d MD5 there is only for reducing filename size. Not all filesystems can handle files named from a C++ generated section name.

str4d added a commit to str4d/flipperzero that referenced this issue Aug 27, 2023
@str4d
Copy link
Contributor Author

str4d commented Aug 27, 2023

@DrZlo13 thanks, that clarifies things (I didn't know objcopy derives section names from filenames in this way - EDIT: I misread due to jetlag; I think this is actually about the build filesystem and has no effect on the FAP?).

I expect that means my current PR is adopting section names from the random file name being chosen by NamedTempFile, which defaults to .tmp<6_RANDOM_CHARS>. Making it deterministic on the section name seems like a better approach.

str4d added a commit to str4d/flipperzero that referenced this issue Aug 27, 2023
@str4d str4d closed this as completed in #99 Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants