Add an option to store redirects as menu router items instead of using hook_init()

Seems like a good idea to me. I'm not sure why you would need to use hook_menu_alter() at all.

This only kind of makes sense to me. I believe this just shifts the performance problem from a database lookup during hook_init to a database lookup when the menu system fires up. I don't think this really gains very much benefit from a performance perspective and may actually be worse.

Is there anything beyond the menu system that would benefit aside from being more logically consistent?

I wonder if there would more benefits from working some kind of caching into the lookup logic. Might be worth running some benchmarks on a large number of paths and redirects to see where (if?) the bottleneck is.

Giving an option seems like feature bloat that is likely to introduce more bugs in the long run.

Putting those redirects (the 5%) into the menu router tables will make the menu lookups on each page slower (I don't know percents, but the more data in the table the slower it is). As I think on it more, perhaps it will be faster because it is doing one query on one dataset - but given that the menu uses some wildcards in the lookups it might also be a slower comparison.

Interesting. I have a feeling that if we switch to menu_router, we are going to see a trickle of bug reports about mysterious slow sites. Those will be sites that have dozens of aliases for same node for some reason. Maybe they like the current year+month in the url, so they alias every month. We could tell those folks to use a different solution, IMO. I kind of like using menu_router for the reason that it saves the DB hit.

Switching to menu_router is a drastic change, and while it would cut the overhead of hook_init(), it also bloats the menu_router table and the caching system associated with that, especially if the number of redirects grows over time.

Still supporting the path_redirect table would have to be an option. I've dealt with migrating several large WordPress blogs to Drupal, requiring thousands of path_redirect entries to preserve permalinks. Obviously all that can't go in the menu_router.

It might be nicer to create a new contrib module with this approach, so existing path_redirect sites can live peacefully. menu_router would only be for savvy site managers that want to skip the per-request overhead.

Maybe I have missed the point, but I did read the comments :)

I still feel that menu_router isn't quite appropriate, so how about this instead (pseudo code):

function path_redirect_init() {
  if ($cache = variable_get('path_redirect_cache', FALSE)) {
    $key = $path . $query . $language;
    if (isset($cache[$key])) {
      // use the cached redirect without running any queries
    }
  }
  else {
    // normal path_redirect table lookup
  }
}

Obviously this only works when the redirects table is very small. When switching to the new mode, the table could be stored in {variables} which has already been fetched from the cache. Alternatively a cache_get() might be used, but that usually means another query unless you're running memcached or the like.

Forgive me if this is too simplistic, I admit I've only just glanced at path_redirect under the hood.

My thoughts:
1) There's already enough bloat in the menu router system. We don't want to grow it even more.
2) With hook_init() approach we could store redirects outside of DB e.g. in memory-based cache backend. With menu router system we can't do that obviously.
3) As a general performance improvement, I suggest to implement a new feature: #791754: Export redirects to webserver rules