Instant Shutdown by only writing changes to gamelist.xml #78

verybadsoldier · 2016-12-19T12:14:18Z

This is a proposal to speedup the shutdown of ES and actually make it shutdown instantly. Usually, ES writes back metadata of all files from all systems to the gamelist.xml. With this PR, ES will keep track of changes to the metadata at runtime and only write back changed entities upon shutdown.

When shutting down there will be a summary of changed gamelist.xmls:

lvl1: 	OptionListComponent too narrow!
lvl2: 	Added/Updated 0 entities in '/home/vbs/.emulationstation/gamelists/3do/gamelist.xml'
lvl2: 	Added/Updated 0 entities in '/home/vbs/.emulationstation/gamelists/amiga/gamelist.xml'
lvl2: 	Added/Updated 1 entities in '/home/vbs/.emulationstation/gamelists/atari7800/gamelist.xml'
lvl2: 	Added/Updated 0 entities in '/home/vbs/.emulationstation/gamelists/atarijaguar/gamelist.xml'
lvl2: 	Added/Updated 0 entities in '/home/vbs/.emulationstation/gamelists/c64/gamelist.xml'
lvl2: 	Added/Updated 0 entities in '/home/vbs/.emulationstation/gamelists/dreamcast/gamelist.xml'
lvl2: 	Added/Updated 0 entities in '/home/vbs/.emulationstation/gamelists/gb/gamelist.xml'
lvl2: 	Added/Updated 0 entities in '/home/vbs/.emulationstation/gamelists/mame-libretro/gamelist.xml'
lvl2: 	Added/Updated 0 entities in '/home/vbs/.emulationstation/gamelists/mastersystem/gamelist.xml'
lvl2: 	Added/Updated 0 entities in '/home/vbs/.emulationstation/gamelists/megadrive/gamelist.xml'
lvl2: 	Added/Updated 0 entities in '/home/vbs/.emulationstation/gamelists/n64/gamelist.xml'
lvl2: 	Added/Updated 0 entities in '/home/vbs/.emulationstation/gamelists/nes/gamelist.xml'
lvl2: 	Added/Updated 0 entities in '/home/vbs/.emulationstation/gamelists/pcengine/gamelist.xml'
lvl2: 	Added/Updated 0 entities in '/home/vbs/.emulationstation/gamelists/psx/gamelist.xml'
lvl2: 	Added/Updated 0 entities in '/home/vbs/.emulationstation/gamelists/retropie/gamelist.xml'
lvl2: 	Added/Updated 0 entities in '/home/vbs/.emulationstation/gamelists/sega32x/gamelist.xml'
lvl2: 	Added/Updated 0 entities in '/home/vbs/.emulationstation/gamelists/segacd/gamelist.xml'
lvl2: 	Added/Updated 0 entities in '/home/vbs/.emulationstation/gamelists/snes/gamelist.xml'
lvl2: 	EmulationStation cleanly shutting down.

I think it should be ok but maybe I should ask some people in the forums if they would test it a bit under real-life conditions. What do you think?

meleu · 2016-12-19T12:41:57Z

I thought that the @jacobfk20 's merged PR #65 did that.

verybadsoldier · 2016-12-19T12:59:49Z

I think it is a bit different: That PR made ES skip files for which the values of all metatags are still at default values. I think it does not help if most of your files actually have been scraped since they won't have default values anymore then.
Anyway, currently on my i5 NUC (>> RPi :)) it takes my ES about 20-30 seconds to shutdown. I have nearly 20 systems and about 30000 (rough estimation) files. 30 seconds might not be that much but still quite annoying when I have to wait for it to boot another OS.
This PR will make ES only write those XML nodes back at shutdown that have actually been touched. So for example you boot ES, play one game (timesPlayed++ for that game), and then shutdown. In this case ES will only change the XML record of that single game.

Maybe I am stupid and I miss something but I don't see (yet) why ES really has to write back all XML records on every shutdown :/

zigurana · 2016-12-19T13:00:15Z

@meleu : I dont think so, that one was only focused at skipping all games without metadata that are not present in the gamelist.xml's to begin with, and skipping trough those (rather than trying to find it). If I look at these changes, @verybadsoldier suggests to keep a ledger of all items that have an explicit change in their metadata, and storing those.

zigurana · 2016-12-19T13:05:25Z

< :-D crossed post with verybadsoldier>
I do wonder though if a single game, halfway through the XML gets an extension of the description field, how will that extra info be inserted? I guess this will trigger a rewrite of the complete xml file, or is this nicely taken care of by the xml library, and done more efficiently to minimize the write activity?
Actually, I am not even sure if the writing is the bottle-neck here.

In any case, the changes look clean to me, what tests do you suggest to try this out?

verybadsoldier · 2016-12-19T13:40:31Z

Yes, you are right. The actual writing to disk won't be the problem. It was triggered once after generating data and it is still triggered once in any case. I also assume the pugixml will always just flush its memory into a new file (and it will not try to modify existing XML elements in the file).
I think the bottleneck was the generation of the XML structure (in memory) using pugixml. ES iterates over all files and for every file it checks if there is a matching XML record (in memory) by iterating over all XML records. Then it deletes that record just to rebuild it from scratch in the next moment (even if it has the same content which will nearly always be the case). I think this rebuilding takes the most time. I profiled it to some extent using layman's timestamp logging but don't have hard numbers anymore.

Too bad I have had no good ideas how to test this extensively. I tested stuff like this:

start ES, shutdown ES. 0 changes in the log. compare XML before/after -> identical
start ES, play a game, shutdown ES. 1 change in the log. compare XML before/after -> one record differs with incremented playcount
same as before but this time manually editing metadata in ES

So maybe you people have some good ideas for another scenarios that could go wrong?

zigurana · 2016-12-19T13:46:23Z

Additional tests I can think of:

Deleting metadata
Deleting a game (via select> edit metadata>delete)
Changing some metadata, reloading / changing a theme to trigger a rebuild of all systemviews, then exit ES
Running the build-in scraper, then exit ES

meleu · 2016-12-19T15:35:37Z

hey guys, now I see the PRs' goal are different.

It would be great to get a faster shutdown. :-)

verybadsoldier · 2016-12-19T16:07:38Z

Okay thanks for the suggestions, I will test those things soon!

Just did one more optimization:
The XML file will only be written if at least one change to the content was done. Hopefully most of the gamelist.xmls won't be written at all then on shutdown. This could be noticeable for RPis running on slow sdcards. But no reason to rewrite the same content to the same XML again anyway.

… speed things up)

verybadsoldier · 2016-12-19T20:44:52Z

Ok, I tried these but I had some problems:

Deleting metadata

I could not find out how to delete metadata :( Is there a way? I only managed to edit it.

Deleting a game (via select> edit metadata>delete)

Delete games did not influence the gamelist.xml. Are games meant to be deleted from gamelist.xml ever?

Changing some metadata, reloading / changing a theme to trigger a rebuild of all systemviews, then exit ES

I always end up having no UI at all anymore when doing this:

Starting ES
Changing a metadata
Changing the theme
-> I get a white whitescreen (an obviously broken theme)

But this also happens without my changes. It does not happen when not editing metadata.

Running the build-in scraper, then exit ES

That worked so far and the gamelist.xml got updated as expected. But some unexpected behavior (for me) showed up while testing this: When scraping, ES writes/updates the gamelist.xml after every scraped game. So when scraping 100 games then the gamelist.xml is written 100 times. The first time with 1 game, then with 2 games etc. etc.
Its no a problem, but just not really efficient. I think when scraping large database then this stuff could really sum up.

joolswills · 2016-12-19T20:52:34Z

thanks for the PR and for the testing.

regarding the last comment and the scraping issue - is this something new due to your changes or an old issue ?

verybadsoldier · 2016-12-19T20:56:53Z

No, it is both unrelated to my changes. I didn't test the scraping issue with the original version (as I did with the first issue) but I could see in the code that updating gamelist.xml (by invoking updateGamelist) for every scraping result is intentional:

void GuiScraperMulti::acceptResult(const ScraperSearchResult& result)
{
	ScraperSearchParams& search = mSearchQueue.front();

	search.game->metadata = result.mdl;
	updateGamelist(search.system);

	mSearchQueue.pop();
	mCurrentGame++;
	mTotalSuccessful++;
	doNextSearch();
}

joolswills · 2016-12-20T14:49:20Z

Thanks for the info.

If you are happy with this, I'm happy to merge. Cheers.

verybadsoldier · 2016-12-20T17:56:57Z

Yes, I think I am happy with it. Quite sure it should be fine. Thanks!

joolswills · 2016-12-20T17:58:24Z

Thanks for the contribution :-)

verybadsoldier · 2016-12-28T22:48:48Z

Just out of curiosity: since no one complained, does it mean no further issues or problems popped up with this? Is the performance advantage as expected also on RPi (actually I was hoping for (near) instant shutdown)?

joolswills · 2016-12-28T22:51:03Z

Many people don't upgrade until they see a new "image" is out, but so far it looks good!

I've not had a chance to do much testing myself though on my own installs, but will let you know.

…-po5-nadenislamarre Recalbox buildroot po5 nadenislamarre

update: pt_BR

verybadsoldier force-pushed the only_save_gamelist_diffs branch from 5c08265 to 9128310 Compare December 19, 2016 16:01

verybadsoldier force-pushed the only_save_gamelist_diffs branch 2 times, most recently from 4cf64b0 to 0a99589 Compare December 19, 2016 16:11

when saving gamelist.xml only save metadata that has been changed (to…

44ea23a

… speed things up)

verybadsoldier force-pushed the only_save_gamelist_diffs branch 2 times, most recently from a0e2594 to 44ea23a Compare December 19, 2016 16:12

joolswills merged commit 74d1bb4 into RetroPie:master Dec 20, 2016

This was referenced Jan 27, 2017

EmulationStation batocera-linux/batocera.linux#58

Closed

Add video view to allow video preview of games in gamelist (port from RetroPie / fieldofcows work) recalbox/recalbox-emulationstation#181

Closed

lubosz pushed a commit to lubosz/EmulationStation that referenced this pull request Feb 29, 2020

Merge pull request RetroPie#78 from nadenislamarre/recalbox-buildroot…

71380a2

…-po5-nadenislamarre Recalbox buildroot po5 nadenislamarre

lubosz pushed a commit to lubosz/EmulationStation that referenced this pull request Feb 29, 2020

Merge pull request RetroPie#78 from jdorigao/pt_BR

1d9d9ce

update: pt_BR

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Instant Shutdown by only writing changes to gamelist.xml #78

Instant Shutdown by only writing changes to gamelist.xml #78

verybadsoldier commented Dec 19, 2016

meleu commented Dec 19, 2016

verybadsoldier commented Dec 19, 2016

zigurana commented Dec 19, 2016

zigurana commented Dec 19, 2016

verybadsoldier commented Dec 19, 2016

zigurana commented Dec 19, 2016

meleu commented Dec 19, 2016

verybadsoldier commented Dec 19, 2016

verybadsoldier commented Dec 19, 2016

joolswills commented Dec 19, 2016

verybadsoldier commented Dec 19, 2016

joolswills commented Dec 20, 2016

verybadsoldier commented Dec 20, 2016

joolswills commented Dec 20, 2016

verybadsoldier commented Dec 28, 2016

joolswills commented Dec 28, 2016

Instant Shutdown by only writing changes to gamelist.xml #78

Instant Shutdown by only writing changes to gamelist.xml #78

Conversation

verybadsoldier commented Dec 19, 2016

meleu commented Dec 19, 2016

verybadsoldier commented Dec 19, 2016

zigurana commented Dec 19, 2016

zigurana commented Dec 19, 2016

verybadsoldier commented Dec 19, 2016

zigurana commented Dec 19, 2016

meleu commented Dec 19, 2016

verybadsoldier commented Dec 19, 2016

verybadsoldier commented Dec 19, 2016

joolswills commented Dec 19, 2016

verybadsoldier commented Dec 19, 2016

joolswills commented Dec 20, 2016

verybadsoldier commented Dec 20, 2016

joolswills commented Dec 20, 2016

verybadsoldier commented Dec 28, 2016

joolswills commented Dec 28, 2016