Re: [aur-dev] [PATCH 5/6] Hack away at misuse of db_connect() in package details view

On Mon, Aug 22, 2011 at 07:18:55PM -0500, Dan McGee wrote:

This drops the MySQL traffic from 55+ actions to under 20. The majority of it is the needless two operations sent on every db_connect, whether the connection is new or not: <connect to database aur> SET NAMES 'utf8' COLLATE 'utf8_general_ci'

Get a handle early, and use it often.

Obviously this performance boost can be applied to other pages as well, but this seemed like the best one to tackle first as it is likely the most viewed page, outside of the crazy amounts of RPC traffic.

Signed-off-by: Dan McGee ---

Before: http://paste.pocoo.org/show/462712/ After: http://paste.pocoo.org/show/462725/

Pretty stark difference.

web/html/packages.php | 25 ++++++++++++------------- web/lib/aur.inc.php | 6 +++--- web/template/header.php | 11 +++++++---- web/template/login_form.php | 2 +- web/template/pkg_comments.php | 3 +-- web/template/pkg_details.php | 15 ++++++--------- 6 files changed, 30 insertions(+), 32 deletions(-)

diff --git a/web/html/packages.php b/web/html/packages.php index dc06c7e..0a7c8d7 100644 --- a/web/html/packages.php +++ b/web/html/packages.php @@ -2,13 +2,14 @@

set_include_path(get_include_path() . PATH_SEPARATOR . '../lib');

-include_once("aur.inc.php"); # access AUR common functions -set_lang(); # this sets up the visitor's language -include_once('pkgfuncs.inc.php'); # package specific functions -check_sid(); # see if they're still logged in +include_once("aur.inc.php"); +include_once('pkgfuncs.inc.php'); +$dbh = db_connect(); +set_lang($dbh); +check_sid($dbh);

# Set the title to the current query if required -if (isset($_GET['ID']) && ($pkgname = pkgname_from_id($_GET['ID']))) { +if (isset($_GET['ID']) && ($pkgname = pkgname_from_id($_GET['ID'], $dbh))) { $title = $pkgname; } else if (!empty($_GET['K'])) { $title = __("Search Criteria") . ": " . $_GET['K']; @@ -18,9 +19,11 @@ if (isset($_GET['ID']) && ($pkgname = pkgname_from_id($_GET['ID']))) {

# Retrieve account type if (isset($_COOKIE["AURSID"])) { - $atype = account_from_sid($_COOKIE["AURSID"]); + $atype = account_from_sid($_COOKIE["AURSID"], $dbh); + $uid = uid_from_sid($SID, $dbh); } else { $atype = ""; + $uid = null; }

# Grab the list of Package IDs to be operated on @@ -78,7 +81,7 @@ if (current_action("do_Flag")) { $output = pkg_change_category($atype); }

-html_header($title); +html_header($title, $dbh); ?>

<?php if ($output): ?> @@ -91,12 +94,8 @@ if (isset($_GET['ID'])) { if (!$_GET['ID'] = intval($_GET['ID'])) { print __("Error trying to retrieve package details.")."<br />\n"; } else { - if (isset($_COOKIE["AURSID"])) { - package_details($_GET['ID'], $_COOKIE["AURSID"]); - } - else { - package_details($_GET['ID'], null); - } + $sid = isset($_COOKIE["AURSID"]) ? $_COOKIE["AURSID"] : null; + package_details($_GET['ID'], $sid, $dbh); } } else { if (!isset($_GET['K']) && !isset($_GET['SB'])) { diff --git a/web/lib/aur.inc.php b/web/lib/aur.inc.php index f432697..4b8a8c8 100644 --- a/web/lib/aur.inc.php +++ b/web/lib/aur.inc.php @@ -320,7 +320,7 @@ function set_lang($dbh=NULL) {

# common header # -function html_header($title="") { +function html_header($title="", $dbh=NULL) { global $_SERVER; global $_COOKIE; global $_POST; @@ -462,9 +462,9 @@ function uid_from_email($email="", $dbh=NULL)

# check user privileges # -function check_user_privileges() +function check_user_privileges($dbh=NULL) { - $type = account_from_sid($_COOKIE['AURSID']); + $type = account_from_sid($_COOKIE['AURSID'], $dbh); return ($type == 'Trusted User' || $type == 'Developer'); }

diff --git a/web/template/header.php b/web/template/header.php index 8313bb3..c56a226 100644 --- a/web/template/header.php +++ b/web/template/header.php @@ -1,4 +1,8 @@ -<?php echo '<?xml version="1.0" encoding="UTF-8"?>'; ?> +<?php echo '<?xml version="1.0" encoding="UTF-8"?>'; +if (!isset($dbh)) { + $dbh = db_connect(); +} +$username = username_from_sid($_COOKIE["AURSID"], $dbh); ?>

You should add some isset() check here to make sure we don't try to access '$_COOKIE["AURSID"]' if it is unset.

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> http://www.w3.org/1999/xhtml" @@ -37,8 +41,8 @@ <li><a href="http://bugs.archlinux.org/index.php?tasks=all&amp;project=2"><?php print __("Bugs"); ?></a></li> <li><a href="http://archlinux.org/mailman/listinfo/aur-general"><?php print __("Discussion"); ?></a></li> <?php if (isset($_COOKIE['AURSID'])): ?> - <?php if (check_user_privileges()): ?><li><a href="tu.php"><?php print __("Trusted User"); ?></a></li><?php endif; ?> - <li><?php print __("My Packages"); ?></a></li> + <?php if (check_user_privileges($dbh)): ?><li><a href="tu.php"><?php print __("Trusted User"); ?></a></li><?php endif; ?> + <li><a href="packages.php?SeB=m&amp;K=<?php print $username; ?>"><?php print __("My Packages"); ?></a></li> <li><a href="pkgsubmit.php"><?php print __("Submit"); ?></a></li> <?php endif; ?> </ul> @@ -59,4 +63,3 @@ foreach ($SUPPORTED_LANGS as $lang => $lang_name) { </div> <!-- Start of main content -->

- diff --git a/web/template/login_form.php b/web/template/login_form.php index b351a27..419e031 100644 --- a/web/template/login_form.php +++ b/web/template/login_form.php @@ -1,7 +1,7 @@ <div id="login_bar" class="pgbox"> <?php if (isset($_COOKIE["AURSID"])) { - print __("Logged-in as: %s", '<b>' . username_from_sid($_COOKIE["AURSID"]) . '</b>'); + print __("Logged-in as: %s", '<b>' . $username . '</b>'); ?> <a href="logout.php">[<?php print __("Logout"); ?>]</a>

See below.

while (list($indx, $carr) = each($comments)) { ?> <div class="comment-header">

@@ -36,7 +35,7 @@ while (list($indx, $carr) = each($comments)) { ?> </div>

10 && !isset($_GET['comments'])) { echo '<div class="pgbox">'; echo '<a href="'. $_SERVER['REQUEST_URI'] . '&comments=all">'. __('Show all %s comments', $count) . '</a>'; diff --git a/web/template/pkg_details.php b/web/template/pkg_details.php index 880a675..599b603 100644 --- a/web/template/pkg_details.php +++ b/web/template/pkg_details.php @@ -1,12 +1,9 @@

You did it again! :) "web/template/pkg_details.php" is included in package_details() where these variables are not visible. Removing these lines breaks all forms that should be visible to logged in users only. Same thing happens with the "$uid" initialization that you removed from "web/template/pkg_comments.php". Adding "global $uid, $atype;" to package_details() might help as a workaround - this needs urgent refactoring anyway. I do not blame you here as our code isn't clear at all and this is almost impossible to spot without having a very close look. Maybe you could try to check if all important parts of the AUR still work and do not issue any warnings after removing some variable initializations next time - just as a tip for the future. Our code is a maze and it's almost impossible to do it right at the first go.

$pkgid = intval($_REQUEST['ID']); if ($uid == $row["MaintainerUID"] or ($atype == "Developer" or $atype == "Trusted User")) {

- $catarr = pkgCategories(); + $catarr = pkgCategories($dbh); $edit_cat = "<form method='post' action='packages.php?ID=".$pkgid."'>\n"; $edit_cat.= "<p>"; $edit_cat.= "<input type='hidden' name='action' value='do_ChangeCategory' />"; @@ -29,7 +26,7 @@ else { }

if ($row["SubmitterUID"]) { - $submitter = username_from_id($row["SubmitterUID"]); + $submitter = username_from_id($row["SubmitterUID"], $dbh); if ($SID) { $submitter = '<a href="account.php?Action=AccountInfo&amp;ID=' . htmlspecialchars($row['SubmitterUID'], ENT_QUOTES) . '">' . htmlspecialchars($submitter) . '</a>'; } @@ -39,7 +36,7 @@ if ($row["SubmitterUID"]) { }

if ($row["MaintainerUID"]) { - $maintainer = username_from_id($row["MaintainerUID"]); + $maintainer = username_from_id($row["MaintainerUID"], $dbh); if ($SID) { $maintainer = '<a href="account.php?Action=AccountInfo&amp;ID=' . htmlspecialchars($row['MaintainerUID'], ENT_QUOTES) . '">' . htmlspecialchars($maintainer) . '</a>'; } @@ -103,8 +100,8 @@ $out_of_date_time = ($row["OutOfDateTS"] == 0) ? $msg : gmdate("r", intval($row[ </p>

- $deps = package_dependencies($row["ID"]); - $requiredby = package_required($row["Name"]); + $deps = package_dependencies($row["ID"], $dbh); + $requiredby = package_required($row["Name"], $dbh);

if (count($deps) > 0 || count($requiredby) > 0) { echo '<p>'; @@ -142,7 +139,7 @@ $out_of_date_time = ($row["OutOfDateTS"] == 0) ? $msg : gmdate("r", intval($row[

# $sources[0] = 'src'; - $sources = package_sources($row["ID"]); + $sources = package_sources($row["ID"], $dbh);

if (count($sources) > 0) {

-- 1.7.6