Ideas for better xmlsitemap performance

The result of xmlsitemap_output() is already cached, and uses the Drupal cache mechanism. If you look at the documentation of drupal_page_footer() (which calls page_set_cache()), you will notice it caches the output creating a database table row in cache_page.
This happens only for the anonymous user, but an XML site map is thought to be used by a search engine, not from a human user which has an account.

MAX() is useless in that case, as the SQL query result is already ordered by lastmod.
Using MAX() in that query would cause the query to return the higher value for lastmod in all the database table, that is not what the code is trying to do.

I'm just posting here all the ideas so I can get some feedback before I try to implement them.

I hope you are suggesting some ideas to improve the project code, and not to change the copy of the project modules you are using.
If you are just trying to modify your own copy, then I will stop to reply your questions.
If you want to get some feedbacks before you submit a patch for the project module, I should tell you that the code you see is not the updated code I am working on; to make a patch, you should wait until I will make public the code I am working on, that is being committed on a branch not associated to any release.

I cannot be precise on that, but I am using a branch that is not associated with a release on purpose.

It has been a mistake to release 6.x-1.x-dev when I was still porting the code from 5.x-2.x-dev. Despite the fact I reported it was not ready for production sites, I had a negative feedback because the release was breaking existing web sites (maybe the implicit use at your own risk sentence was not understood).

As I need to commit the changes to the code to Drupal repository (in this way, my PC could have problems but I would not loose the code I developed), I will not make a release until I am sure the code is stable enough.

#5: i'm using the original xmlsitemap code version and of course i was talking about adding patches to it :) so don't worry.

That is exactly what I was talking of; 6.x-0.x-dev version will not get any fixes, or code changes. If you want to change it, you do it at your risk.
If you are referring to 6.x-1.x-dev, then any patches to that code will become obsolete once I will make public the work I am doing.

I don't want to be misunderstood. Any help is always welcome, and I thank you for your observations.
The fact I got negative feedbacks made me more prudent. That is the main reason I didn't associated any release with the branch I am committing the code in.

@#2: the cache system adopted for Drupal is only valid for the anonymous users; it doesn't conflict with the memory cache you are using that is valid for any user accessing to a Drupal-powered system.
Using drupal_page_footer() gives us the guarantee to have code always updated. If the cache method used by Drupal will be changed, we are sure that the project will use the updated code without to change anything.

I think that when Drupal core code offers some functions, those should be used as much as possible. The only case when the core functions should not be used is when they don't fit in the code we are writing, or when using them would force us to use a long workaround.

cache_get(), and cache_set() are called by all the modules that need to cache their data. I don't then see any issues on using a set of functions that is thought to be used in such cases.
Is there any reason a XML site map should not be cached?

Right. Is there a reason XML Sitemap should be treated differently?

All the modules which cache their data use cache_set()/cache_get(), and none of them is able to know if the cache will be kept in a database table or in memory.
I think that it should be core functions to decide when to use memory cache, or database cache; third party modules have just the choice to cache their data, or to not cache their data.

The cached data is not plain data, but it's XML data that is not even serialized. Cache is not used to contain complex data, but to reduce the time needed to reply to a page request.

That said, I understand your point. I think that the cached data must be different; at the actual state, xmlsitemap.module caches the final result, while it should cache the data used to generate that result.

I don't like to cache the site map chunks in files, and I am not sure it will be possible after I change the code like I am thinking.
My idea was to introduce a parameter to delay the insertion of a link in the site map. In this way, i.e., a link of a node would not inserted immediately, but X seconds after it has been modified. This would solve the problem with nodes that have short life (they could be spam nodes that weren't blocked) which causes Google Webmaster Tools to report many 404 errors (in the case the short life node appeared in the site map, and then it has been deleted from the site, and the site map).

I will see about generating the site map only on execution of CRON tasks (but that is not possible, if the site map is not going to be saved in a file). The module needs to implement an option to enable/disable that, as not all the web sites have the possibility to execute CRON tasks.

I am setting the status to postponed.
Thanks for your report.

The module which will handle that feature will be xmlsitemap.module, which will check the data saved in the xmlsitemap database table from other modules.

To cache the site map in a file would not resolve the memory problem.
As the file is a cache, the code should first load the content of the file in memory, and then output its content. Being the file loaded in memory, it would take as much memory as loading it in a memory cache.
If then the code would load the file line by line, there would not be an increment of speed from the actual code implemented, without the call to drupal_page_footer().

The problem would be present also using a custom database table to cache the site map, because also in that case the cached data should be loaded in memory before to be output.

XML Sitemap must work in multi-site, or multi-language sites too. The implemented code must take in consideration these cases.
The only solution I can find is to create settings for the cache, which would permit to activate the cache. In this way, who has a site map that cannot be cached because this causes memory problems can at least disable it.

So far, it's not clear if the #231420: Allowed memory size exhausted issue is caused by a memory cache being used. That is the reason I asked the question in that issue report.

I think you should have waited to make public your patch, at least for a good reason.

I already explained in other reports like the project code is being modified (in particular, I already said that the project modules will support the multi-language sites), and I also explained here that there is a branch I am developing that is not associated to any releases (therefore, if you don't look into CVS repository you cannot know how the code is being modified).

In particular, the multi-language support implicates that xmlsitemap.module will produce a different site map for every languages activated on a Drupal web site.
This is because a page with a language associated to it can have a URL that is on a different domain; even if this would not be the case, a site map at, i.e., /it/sitemap.xml cannot contain URLs starting with /es/. This happens because, on a Drupal site with locale.module active, the menu callbacks defined from the modules can be associated to different URLs (in the specific, the menu callback which outputs the site map can have URLs like http://example.com/it/sitemap.xml, http://example.com/es/sitemap.xml, http://example.com/eo/sitemap.xml).

First you open this report to suggest any ideas to optimize the code, and then you start to attach patches to different reports not even listening to what one of the maintainers of the project is telling you.

I am setting this report like duplicate, as the same topic is now debated in #329063: Allowed memory size exhausted.