NFC: Cache plugin name not full path, saves some RAM

[Willy-JL]

What's new

• Cache remembers plugins names instead of full path, which saves some RAM

Verification

• Reading and opening saved files works as intended, plugins load correctly

Checklist (For Reviewer)

• PR has description of feature/bug or link to Confluence/Jira task
• Description contains actions to verify feature/bugfix
• I've built this code, uploaded it to the device and verified feature/bugfix

[Willy-JL]

surprisingly, on CFW with 29 plugins, this saved about 1.5kb of RAM, and resulted in less fragmentation:

cache path:

cache name:

[CookiePLMonster]

Trimming the suffix does not save you any memory (as the allocation is not getting shrunk), so is there a point in doing so and then re-concatenating it?

[Willy-JL]

Yes. Because the path with suffix is being saved in the cache. It's concatenating again when loading from file, but if it wasn't removed it would be kept around even in the cache. get_plugin() will construct the path that get_next_plugin() had trimmed, but load_cache() is calling get_next_plugin() in a loop and saving into cache with the resulting name from get_next_plugin(). So instead of full path with parent directories and name suffix, it only keeps in cache the name, which is the only varying value between the plugin paths.

[Willy-JL]

It doesn't really save memory during the loop that's true, but it's preventing redundant data from being kept in the cache

[CookiePLMonster]

Sure, but if you are not saving any memory by trimming the suffix, then you are only wasting cycles to reconstruct it and not gaining anything in return. Unless I am missing something.

[Willy-JL]

It is saving memory. Sure you can debate if it's better to save ram or a few clock cycles, but there is definitely a ram save here. Look at line 184/191, instead of the full path, it's saving just the name into the cache array. The purpose of cache is knowing which plugins contain which type of functionality, so when going to parse a nfc tag / saved file, it doesn't need to load every single plugin, but only relevant ones. As it stands currently, the cache array contains paths.

Cache array before:

• index 0:
  • path: /data/plugins/plugin1_parser.fal
  • features
• index 1:
  • path: /data/plugins/plugin2_parser.fal
  • features
• index 2:
  • path: /data/plugins/plugin3_parser.fal
  • features

This is redundant data, instead the can just save the names.

Cache array with this PR:

• index 0:
  • name: plugin1
  • features
• index 1:
  • name: plugin2
  • features
• index 2:
  • name: plugin3
  • features

[Willy-JL]

You're looking only at get_next_plugin() and get_plugin(), where one trims the path and the other reconstructs it. This is not the memory save itself, this is what enables the memory save, because load_cache() is then saving the trimmed name that get_next_plugin() made into the cache, and so it saves less data, only relevant one.

[CookiePLMonster]

OK, I see it more clearly now - instance->file_name is being modified in place, but later when saving it to cache the string gets copied over without the suffix, hence saving memory.

[skotopes]

@gornekich can you take a look?

[gornekich]

Hi @Willy-JL , thanks for PR!
Everything is fine, but I would prefer to remove file_path form NfcSupportedCardsLoadContext. Please, consider the patch with this change
remove_file_path.txt

[Willy-JL]

that looks perfect, thank you!

[gornekich]

Thanks!