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

Use liblcf string and array types #2280

Merged
merged 21 commits into from
Sep 1, 2020
Merged

Use liblcf string and array types #2280

merged 21 commits into from
Sep 1, 2020

Conversation

mateofio
Copy link
Contributor

Depends on: EasyRPG/liblcf#379

See liblcf PR for details.

@mateofio mateofio added Refactor Has PR Dependencies This PR depends on another PR Performance Related to improvements on memory, less CPU or disk usage, battery savings, etc. labels Aug 16, 2020
src/cache.cpp Outdated
bool transparent, const uint32_t flags) {
const auto key = MakeHashKey(folder_name, filename, transparent);

auto it = cache.find(key);

if (it == cache.end()) {
const std::string path = FileFinder::FindImage(folder_name, filename);
// FIXME: STRING_VIEW string copies here
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this fixme for later PRs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I didn't fully StringView'ify filefinder.

Copy link
Member

Choose a reason for hiding this comment

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

I would prefer it when you don't touch the FileFinder too much otherwise my next big FS PR will be terrible to rebase.

}
} }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Highlighting these ugly hacks to get fmtlib working without needing to include <fmt/format.h> everywhere

Copy link
Member

Choose a reason for hiding this comment

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

any hack that doesn't increase buildtime by 100% because of libfmt is good.


namespace lcf {
// FIXME: This is hacky, but unfortunately nowhere else convenient to put this to be able to print DBString ...
inline fmt::basic_string_view<char> to_string_view(const lcf::DBString& s) {
return to_string_view(StringView(s));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

And this one, which is worse because we shouldn't even be including DBString here, but nowhere else to put this..

@mateofio mateofio marked this pull request as draft August 21, 2020 21:40
@mateofio mateofio force-pushed the string branch 2 times, most recently from 723ce46 to 3c3ceac Compare August 26, 2020 03:58
@mateofio mateofio marked this pull request as ready for review August 26, 2020 04:42
@mateofio mateofio changed the title Use liblcf string types Use liblcf string and array types Aug 26, 2020
@Ghabry Ghabry added this to the 0.6.3 milestone Sep 1, 2020
Copy link
Member

@Ghabry Ghabry left a comment

Choose a reason for hiding this comment

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

Some more numbers for the title screen. The memory usage of the map is hard to determine but likely also saves some KB to MB there depending on the events

  • Alter Aila Genesis: 297 -> 185
  • Heros Realm: 297 -> 177
  • Il mito: 120 -> 92
  • Final Tear 3: 146 -> 93
  • VH1: 101 -> 74
  • TTHW: 88 -> 58
  • DQ4: 66 -> 49
  • Frozen Triggers: 60 -> 48
  • 7thJojo: 54 -> 40
  • Lakria Legends: 53 -> 40
  • Sternenkindsaga: 40 -> 31
  • Pokekon Eevee: 36 -> 32
  • Wolfenhain: 29 -> 25
  • Vampires Dawn II: 21 -> 20
  • ゆめ2っきver0.113a: 24 -> 21
  • DEEP 8 Demo: 26 -> 23
  • Unterwegs in Düsterburg: 18 -> 17
  • Yume Nikki: 15 -> 14,5
  • Ib: 13,9 -> 13,8

@Ghabry Ghabry removed the Has PR Dependencies This PR depends on another PR label Sep 1, 2020
@Ghabry
Copy link
Member

Ghabry commented Sep 1, 2020

Some more numbers for reduced memory usage in MB on the title screen. The memory usage of the map is hard to determine but likely also saves some KB to MB there depending on the events

  • Alter Aila Genesis: 297 MB -> 185 MB
  • Heros Realm: 297 -> 177
  • Il mito: 120 -> 92
  • Final Tear 3: 146 -> 93
  • VH1: 101 -> 74
  • TTHW: 88 -> 58
  • DQ4: 66 -> 49
  • Frozen Triggers: 60 -> 48
  • 7thJojo: 54 -> 40
  • Lakria Legends: 53 -> 40
  • Sternenkindsaga: 40 -> 31
  • Pokekon Eevee: 36 -> 32
  • Wolfenhain: 29 -> 25
  • Vampires Dawn II: 21 -> 20
  • ゆめ2っきver0.113a: 24 -> 21
  • DEEP 8 Demo: 26 -> 23
  • Unterwegs in Düsterburg: 18 -> 17
  • Yume Nikki: 15 -> 14,5
  • Ib: 13,9 -> 13,8

@Ghabry Ghabry merged commit d1e3602 into EasyRPG:master Sep 1, 2020
@mateofio mateofio deleted the string branch September 1, 2020 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Performance Related to improvements on memory, less CPU or disk usage, battery savings, etc. Refactor
Development

Successfully merging this pull request may close these issues.

3 participants