Cache list info in Sympa::Scenario::verify to reduce overhead of pinfo…

[mpkut]

Our site has approximately 6000 lists and 600,000 subscribers. In order to do performance testing of our upgrade to Sympa 6.2, we created a demo server and loaded it with that many lists and subscribers. We found that the lists index page could take over a minute to load.

Profiling with NYTProf indicated that copying the %pinfo hash in Sympa::Robot::list_params() was the primary source of the time spent inside do_lists(), once wwsympa.fcgi was initialized.

Since Sympa::Scenario::verify() was the primary caller of Sympa::Robot::list_params during the list index, that code seemed to be a likely place for some caching. Robot parameters can change (say when topics are updated), so the cached data is updated every $picache_refresh seconds.

With this patch, lists index page performance with our demo Sympa 6.2.22 installation is approximately 3 seconds, instead of over 60. And it is comparable to our existing installation's performance.

The before and after profiling data is attached as nytprof-picache.zip:

• before: nytprof-6.2.22
• after: nytprof-6.2.22-picache

The profiling data represents 5 successive requests to /sympa/lists to a single instance of wwsympa.fcgi running on our loaded demo server. We selected 5 requests instead of just 1 in order to reduce the impact of the significant overhead of simply starting wwsympa at all.

While I am not familiar enough with Sympa's design to be sure this is the very best way to approach this performance issue, this is an attempt to avoid breaking things while at the same time making things run acceptably fast.

--mic--

nytprof-picache.zip

[ikedas]

@mpkut, thanks for valuable inputs. 👍

I agree that caching result of list_params() looks best solution of live issue. If no objection by others, I'll merge your PR in a few weeks.

I think more intrinsic solution is making "robot" be object: By such change, robot object should load configuration information including %ListDef::pinfo (and topics.conf) at once when it is instantiated, and won't need reloading.

[mpkut]

Thanks for considering this patch. It does seem like it would be architecturally best to make the pinfo array an attribute of a robot object. Hopefully the approach in this patch is sufficient and avoids doing much harm in the meantime.