diff options
author | Yotam Barnoy | 2010-11-05 13:24:57 +0000 |
---|---|---|
committer | Yotam Barnoy | 2010-11-05 13:24:57 +0000 |
commit | a6bee87990d3fd36cbb8e63716990a46fe00402c (patch) | |
tree | 5240814d46e4e0f3a59704354e4a643be3dc6108 /base | |
parent | 6b88fd44c0fca0b8439f820bceac808562697d0d (diff) | |
download | scummvm-rg350-a6bee87990d3fd36cbb8e63716990a46fe00402c.tar.gz scummvm-rg350-a6bee87990d3fd36cbb8e63716990a46fe00402c.tar.bz2 scummvm-rg350-a6bee87990d3fd36cbb8e63716990a46fe00402c.zip |
PLUGINS: improved one-at-a-time plugin code
I reduced memory fragmentation using 2 principles: Plugins should be loaded for as little time as possible, and long lasting memory allocations should be allocated before plugins are loaded. There might still be a little fragmentation left.
Note that command line settings that require plugins to be loaded don't work yet, but they didn't work (properly) before either.
svn-id: r54097
Diffstat (limited to 'base')
-rw-r--r-- | base/main.cpp | 14 | ||||
-rw-r--r-- | base/plugins.cpp | 157 | ||||
-rw-r--r-- | base/plugins.h | 14 |
3 files changed, 97 insertions, 88 deletions
diff --git a/base/main.cpp b/base/main.cpp index 8928396642..a32589e701 100644 --- a/base/main.cpp +++ b/base/main.cpp @@ -107,7 +107,7 @@ static const EnginePlugin *detectPlugin() { printf("%s", " Looking for a plugin supporting this gameid... "); #if defined(ONE_PLUGIN_AT_A_TIME) && defined(DYNAMIC_MODULES) - GameDescriptor game = EngineMan.findGameOnePlugAtATime(gameid, &plugin); + GameDescriptor game = EngineMan.findGameOnePluginAtATime(gameid, &plugin); #else GameDescriptor game = EngineMan.findGame(gameid, &plugin); #endif @@ -216,6 +216,11 @@ static Common::Error runGame(const EnginePlugin *plugin, OSystem &system, const // Run the engine Common::Error result = engine->run(); +#if defined(ONE_PLUGIN_AT_A_TIME) && defined(DYNAMIC_MODULES) + // do our best to prevent fragmentation by unloading as soon as we can + PluginManager::instance().unloadPluginsExcept(PLUGIN_TYPE_ENGINE, NULL, false); +#endif + // Inform backend that the engine finished system.engineDone(); @@ -343,7 +348,7 @@ extern "C" int scummvm_main(int argc, const char * const argv[]) { #if defined(ONE_PLUGIN_AT_A_TIME) && defined(DYNAMIC_MODULES) // Only load non-engine plugins and first engine plugin initially in this case. - PluginManager::instance().loadFirstPlugin(); + PluginManager::instance().loadNonEnginePluginsAndEnumerate(); #else // Load the plugins. PluginManager::instance().loadPlugins(); @@ -363,6 +368,7 @@ extern "C" int scummvm_main(int argc, const char * const argv[]) { // config file and the plugins have been loaded. Common::Error res; + // TODO: deal with settings that require plugins to be loaded if ((res = Base::processSettings(command, settings)) != Common::kArgumentNotProcessed) return res; @@ -431,9 +437,7 @@ extern "C" int scummvm_main(int argc, const char * const argv[]) { // PluginManager::instance().unloadPlugins(); -#if defined(ONE_PLUGIN_AT_A_TIME) && defined(DYNAMIC_MODULES) - PluginManager::instance().loadFirstPlugin(); -#else +#if !defined(ONE_PLUGIN_AT_A_TIME) PluginManager::instance().loadPlugins(); #endif } else { diff --git a/base/plugins.cpp b/base/plugins.cpp index d2fad34579..41e213a53a 100644 --- a/base/plugins.cpp +++ b/base/plugins.cpp @@ -305,7 +305,6 @@ DECLARE_SINGLETON(PluginManager) PluginManager::PluginManager() { // Always add the static plugin provider. addPluginProvider(new StaticPluginProvider()); - _skipStaticPlugs = false; } PluginManager::~PluginManager() { @@ -324,58 +323,60 @@ void PluginManager::addPluginProvider(PluginProvider *pp) { _providers.push_back(pp); } -void PluginManager::loadFirstPlugin() { //TODO: rename? It's not quite clear that this loads all non-engine plugins and first engine plugin. - unloadPluginsExcept(PLUGIN_TYPE_ENGINE, NULL); - _allPlugs.clear(); +// +// This should only be run once +void PluginManager::loadNonEnginePluginsAndEnumerate() { + unloadPlugins(); + _allEnginePlugins.clear(); + + // We need to resize our pluginsInMem list to prevent fragmentation + // Otherwise, as soon as we add our 1 engine plugin (which is all we'll have in memory at a time) + // We'll get an allocation in memory that will never go away + _pluginsInMem[PLUGIN_TYPE_ENGINE].resize(2); // more than we need + for (ProviderList::iterator pp = _providers.begin(); pp != _providers.end(); ++pp) { - if ((_skipStaticPlugs && (*pp)->isFilePluginProvider()) || !_skipStaticPlugs) { - PluginList pl((*pp)->getPlugins()); - for (PluginList::iterator p = pl.begin(); p != pl.end(); ++p) { - _allPlugs.push_back(*p); - } - } - } - - _nonEnginePlugs = 0; - _skipStaticPlugs = true; //Only need to load static plugins once. - - _currentPlugin = _allPlugs.begin(); - - if (_allPlugs.empty()) { - return; //return here if somehow there are no plugins to load. - } + PluginList pl((*pp)->getPlugins()); + for (PluginList::iterator p = pl.begin(); p != pl.end(); ++p) { + // To find out which are engine plugins, we have to load them. This is inefficient + // Hopefully another way can be found (e.g. if the music plugins are all static, + // we can use only the static provider + if ((*p)->loadPlugin()) { + if ((*p)->getType() == PLUGIN_TYPE_ENGINE) { + (*p)->unloadPlugin(); // to prevent fragmentation + _allEnginePlugins.push_back(*p); + } else { // add non-engine plugins to the 'in-memory' list + // these won't ever get unloaded (in this implementation) + addToPluginsInMemList(*p); + } + } + } + } +} - //this loop is for loading all non-engine plugins and the first engine plugin. - for (; _currentPlugin != _allPlugs.end(); ++_currentPlugin) { - if (!tryLoadPlugin(*_currentPlugin)) - continue; +void PluginManager::loadFirstPlugin() { + unloadPluginsExcept(PLUGIN_TYPE_ENGINE, NULL, false); - // TODO: This assumes all non-engine plugins will precede the first engine plugin! - if ((*_currentPlugin)->getType() == PLUGIN_TYPE_ENGINE) + // let's try to find one we can load + for (_currentPlugin = _allEnginePlugins.begin(); _currentPlugin != _allEnginePlugins.end(); ++_currentPlugin) { + if ((*_currentPlugin)->loadPlugin()) { + addToPluginsInMemList(*_currentPlugin); break; - - _nonEnginePlugs++; + } } - - return; } bool PluginManager::loadNextPlugin() { - if (_nonEnginePlugs == _allPlugs.size()) - return false; //There are no Engine Plugins in this case. + unloadPluginsExcept(PLUGIN_TYPE_ENGINE, NULL, false); - // To ensure only one engine plugin is loaded at a time, we unload all engine plugins before trying to load a new one. - unloadPluginsExcept(PLUGIN_TYPE_ENGINE, NULL); - - ++_currentPlugin; - for (; _currentPlugin != _allPlugs.end(); ++_currentPlugin) - if (tryLoadPlugin(*_currentPlugin)) + for (++_currentPlugin; _currentPlugin != _allEnginePlugins.end(); ++_currentPlugin) { + if ((*_currentPlugin)->loadPlugin()) { + addToPluginsInMemList(*_currentPlugin); return true; - - loadFirstPlugin(); - return false; + } + } + return false; // no more in list } void PluginManager::loadPlugins() { @@ -392,19 +393,20 @@ void PluginManager::unloadPlugins() { unloadPluginsExcept((PluginType)i, NULL); } -void PluginManager::unloadPluginsExcept(PluginType type, const Plugin *plugin) { +void PluginManager::unloadPluginsExcept(PluginType type, const Plugin *plugin, bool deletePlugin /*=true*/) { Plugin *found = NULL; - for (PluginList::iterator p = _plugins[type].begin(); p != _plugins[type].end(); ++p) { + for (PluginList::iterator p = _pluginsInMem[type].begin(); p != _pluginsInMem[type].end(); ++p) { if (*p == plugin) { found = *p; } else { (*p)->unloadPlugin(); - delete *p; + if (deletePlugin) + delete *p; } } - _plugins[type].clear(); + _pluginsInMem[type].clear(); if (found != NULL) { - _plugins[type].push_back(found); + _pluginsInMem[type].push_back(found); } } @@ -412,26 +414,7 @@ bool PluginManager::tryLoadPlugin(Plugin *plugin) { assert(plugin); // Try to load the plugin if (plugin->loadPlugin()) { - // The plugin is valid, see if it provides the same module as an - // already loaded one and should replace it. - bool found = false; - PluginList::iterator pl = _plugins[plugin->getType()].begin(); - while (!found && pl != _plugins[plugin->getType()].end()) { - if (!strcmp(plugin->getName(), (*pl)->getName())) { - // Found a duplicated module. Replace the old one. - found = true; - delete *pl; - *pl = plugin; - debug(1, "Replaced the duplicated plugin: '%s'", plugin->getName()); - } - pl++; - } - - if (!found) { - // If it provides a new module, just add it to the list of known plugins. - _plugins[plugin->getType()].push_back(plugin); - } - + addToPluginsInMemList(plugin); return true; } else { // Failed to load the plugin @@ -440,15 +423,38 @@ bool PluginManager::tryLoadPlugin(Plugin *plugin) { } } +void PluginManager::addToPluginsInMemList(Plugin *plugin) { + bool found = false; + // The plugin is valid, see if it provides the same module as an + // already loaded one and should replace it. + + PluginList::iterator pl = _pluginsInMem[plugin->getType()].begin(); + while (!found && pl != _pluginsInMem[plugin->getType()].end()) { + if (!strcmp(plugin->getName(), (*pl)->getName())) { + // Found a duplicated module. Replace the old one. + found = true; + delete *pl; + *pl = plugin; + debug(1, "Replaced the duplicated plugin: '%s'", plugin->getName()); + } + pl++; + } + + if (!found) { + // If it provides a new module, just add it to the list of known plugins in memory. + _pluginsInMem[plugin->getType()].push_back(plugin); + } +} + // Engine plugins #include "engines/metaengine.h" DECLARE_SINGLETON(EngineManager) -GameDescriptor EngineManager::findGameOnePlugAtATime(const Common::String &gameName, const EnginePlugin **plugin) const { +GameDescriptor EngineManager::findGameOnePluginAtATime(const Common::String &gameName, const EnginePlugin **plugin) const { GameDescriptor result; - //PluginManager::instance().loadFirstPlugin(); + PluginManager::instance().loadFirstPlugin(); do { result = findGame(gameName, plugin); if (!result.gameid().empty()) { @@ -468,14 +474,14 @@ GameDescriptor EngineManager::findGame(const Common::String &gameName, const Eng EnginePlugin::List::const_iterator iter; - for (iter = plugins.begin(); iter != plugins.end(); ++iter) { - result = (**iter)->findGame(gameName.c_str()); - if (!result.gameid().empty()) { - if (plugin) - *plugin = *iter; - return result; - } + for (iter = plugins.begin(); iter != plugins.end(); ++iter) { + result = (**iter)->findGame(gameName.c_str()); + if (!result.gameid().empty()) { + if (plugin) + *plugin = *iter; + return result; } + } return result; } @@ -484,6 +490,7 @@ GameList EngineManager::detectGames(const Common::FSList &fslist) const { EnginePlugin::List plugins; EnginePlugin::List::const_iterator iter; #if defined(ONE_PLUGIN_AT_A_TIME) && defined(DYNAMIC_MODULES) + PluginManager::instance().loadFirstPlugin(); do { #endif plugins = getPlugins(); diff --git a/base/plugins.h b/base/plugins.h index 5472ab2fce..fc8adbf0d4 100644 --- a/base/plugins.h +++ b/base/plugins.h @@ -301,17 +301,14 @@ protected: class PluginManager : public Common::Singleton<PluginManager> { typedef Common::Array<PluginProvider *> ProviderList; private: - PluginList _plugins[PLUGIN_TYPE_MAX]; + PluginList _pluginsInMem[PLUGIN_TYPE_MAX]; ProviderList _providers; - PluginList _allPlugs; + PluginList _allEnginePlugins; PluginList::iterator _currentPlugin; - bool _skipStaticPlugs; - - uint _nonEnginePlugs; - bool tryLoadPlugin(Plugin *plugin); + void addToPluginsInMemList(Plugin *plugin); friend class Common::Singleton<SingletonBaseType>; PluginManager(); @@ -321,14 +318,15 @@ public: void addPluginProvider(PluginProvider *pp); + void loadNonEnginePluginsAndEnumerate(); void loadFirstPlugin(); bool loadNextPlugin(); void loadPlugins(); void unloadPlugins(); - void unloadPluginsExcept(PluginType type, const Plugin *plugin); + void unloadPluginsExcept(PluginType type, const Plugin *plugin, bool deletePlugin = true); - const PluginList &getPlugins(PluginType t) { return _plugins[t]; } + const PluginList &getPlugins(PluginType t) { return _pluginsInMem[t]; } }; #endif |