1
\$\begingroup\$

I need help to speed up this code. It works, but is really slow (updates 2 items/second). Any tips on how to speed it up?

The script functions like this:

  1. Break up an email to username and domain
  2. If not already in database, add username to username table and the domain to domain table
  3. Add the username ID and domain ID to the email table (i.e. "username.ID 1"@"domain.ID 1")
  4. Same thing with password
  5. Add a relationship to the username, domain, email and password to each other and to the source table
  6. Delete the old row
  7. Print each email/password
  8. Print final results

My MySQL database layout:

  • domain: ID - domain (i.e. hotmail.com)
  • username: ID - username (i.e. myname)
  • email: ID - username - domain (i.e. username.ID - domain.ID)
  • source: ID - source (i.e. old_userlist)
  • relationships: ID - username - email - password - source
  • old_userlist: ID - email - password

//conn

$servername = "xx";
$username = "xx";
$password = "xx";
$dbname = "xx";

// Create connection
$conn = new mysqli($servername, $username, $password, $dbname);
// Check connection
if ($conn->connect_error) {
    die("Connection failed: " . $conn->connect_error);
}

// List all email
$sql = "SELECT * FROM old_userlist ORDER BY id ASC LIMIT 1000";
$result = $conn->query($sql);

if ($result->num_rows > 0) {
    // output data of each row
    while($row = $result->fetch_assoc()) {
      //Get id of original row
      $original_email_id = $row["ID"];
      //count
      $domain_name_total_count = $domain_name_total_count + 1;
      $password_total_count = $password_total_count + 1;

      $username_id = 0;
      $domain_id = 0;
      $email_id = 0;

      $row_email = $row["email"];
      $row_password = $row["password"];

        //break the string up around the "@" character in $row_email
        $row_email_parts = explode("@",$row_email);
        $row_email_username = $row_email_parts['0'];
        $row_email_domain = $row_email_parts['1'];

// ############ USERNAME

        //check if domain name exists in the database
        $sql = "SELECT * FROM username WHERE username LIKE ('$row_email_username')";
        $result_emailadress = $conn->query($sql);

          if ($result_emailadress->num_rows > 0) {
            //count duplicates
            $username_duplicate_count = $username_duplicate_count + 1;
            //get usernamne id
            while($row_username = $result_emailadress->fetch_assoc()) {
              $username_id = $row_username["ID"];
            }

          } else {
            //count new
            $username_new_count = $username_new_count + 1;
            //Add domains to database
            $sql= "INSERT INTO username (username) VALUES ('$row_email_username')";
            $insert = $conn->query($sql);
            $username_id = $conn->insert_id;

          } // /add username

// ############ DOMAIN

          //check if domain name exists in the database
          $sql = "SELECT * FROM domain WHERE domain LIKE ('$row_email_domain')";
          $result_domain = $conn->query($sql);

            if ($result_domain->num_rows > 0) {
              //count duplicates
              $domain_name_duplicate_count = $domain_name_duplicate_count + 1;
              //get domain id
              while($row_domain = $result_domain->fetch_assoc()) {
                $domain_id = $row_domain["ID"];
              }
            } else {
              //count new
              $domain_name_new_count = $domain_name_new_count + 1;
              //Add domains to database
              $sql= "INSERT INTO domain (domain) VALUES ('$row_email_domain')";
              $insert_domain = $conn->query($sql);
              $domain_id = $conn->insert_id;

            } // /add domain

// ############ CREATE EMAIL entity

        //check if email name exists in the database
        $sql = "SELECT * FROM email_address WHERE u LIKE ('$username_id') AND d LIKE ('$domain_id')";
        $result_emailadress = $conn->query($sql);

          if ($result_emailadress->num_rows > 0) {
            //count duplicates
            $email_adress_duplicate_count = $email_adress_duplicate_count + 1;

            while($row_email_new = $result_emailadress->fetch_assoc()) {
              $email_id = $row_email_new["ID"];
            }

          } else {
            //count new
            $email_adress_new_count = $email_adress_new_count + 1;
            //Add email to database
            $sql= "INSERT INTO email_address (u, d) VALUES ('$username_id','$domain_id')";
            $insert_email = $conn->query($sql);
            $email_id = $conn->insert_id;

          } // /add email


// ############ CREATE PASSWORD entity

        //check if email name exists in the database
        $sql = "SELECT * FROM password WHERE p LIKE ('$row_password')";
        $result_password = $conn->query($sql);

          if ($result_password->num_rows > 0) {
            //count duplicates
            $password_duplicate_count = $password_duplicate_count + 1;
            while($row_password_new = $result_password->fetch_assoc()) {
              $password_id = $row_password_new["ID"];
            }
          } else {
            //count new
            $password_new_count = $password_new_count + 1;
            //Add email to database
            $sql= "INSERT INTO password (p) VALUES ('$row_password')";
            $insert_password = $conn->query($sql);
            $password_id = $conn->insert_id;

          } // /password

// // ############ CREATE relationship

$source_id = "1"; // old_userlist
//check if username connected to source.
$sql = "SELECT * FROM relationship WHERE u LIKE ('$username_id') AND s LIKE ('$source_id')";
$result_relationship_user_source = $conn->query($sql);

  if ($result_relationship_user_source->num_rows > 0) {
    //count duplicates
    $relationship_user_source_duplicate_count = $relationship_user_source_duplicate_count + 1;
  } else {
    //count new
    $relationship_user_source_new_count = $relationship_user_source_new_count + 1;
    //Add email to database
    $sql= "INSERT INTO relationship (u,s) VALUES ('$username_id','$source_id')";
    $insert_relationship_user_source = $conn->query($sql);
  } // /relationship username-source

  //check if email connected to source.
  $sql = "SELECT * FROM relationship WHERE e LIKE ('$email_id') AND s LIKE ('$source_id')";
  $result_relationship_email_source = $conn->query($sql);

    if ($result_relationship_email_source->num_rows > 0) {
      //count duplicates
      $relationship_email_source_duplicate_count = $relationship_email_source_duplicate_count + 1;
    } else {
      //count new
      $relationship_email_source_new_count = $relationship_email_source_new_count + 1;
      //Add email to database
      $sql= "INSERT INTO relationship (e,s) VALUES ('$email_id','$source_id')";
      $insert_relationship_email_source = $conn->query($sql);
    } // /relationship username-source

    //check if password connected to source.
    $sql = "SELECT * FROM relationship WHERE p LIKE ('$password_id') AND s LIKE ('$source_id')";
    $result_relationship_password_source = $conn->query($sql);

      if ($result_relationship_password_source->num_rows > 0) {
        //count duplicates
        $relationship_password_source_duplicate_count = $relationship_password_source_duplicate_count + 1;
      } else {
        //count new
        $relationship_password_source_new_count = $relationship_password_source_new_count + 1;
        //Add email to database
        $sql= "INSERT INTO relationship (p,s) VALUES ('$password_id','$source_id')";
        $insert_relationship_email_source = $conn->query($sql);
      } // /relationship password-source

      //check if email connected to password.
      $sql = "SELECT * FROM relationship WHERE p LIKE ('$password_id') AND e LIKE ('$email_id')";
      $result_relationship_email_password = $conn->query($sql);

        if ($result_relationship_email_password->num_rows > 0) {
          //count duplicates
          $relationship_email_password_duplicate_count = $relationship_email_password_duplicate_count + 1;
        } else {
          //count new
          $relationship_email_password_new_count = $relationship_email_password_new_count + 1;
          //Add email to database
          $sql= "INSERT INTO relationship (p,e) VALUES ('$password_id','$email_id')";
          $insert_relationship_email_password = $conn->query($sql);
        } // /relationship email-password


// ############ DELETE ORIGINAL entity
    $sql= "DELETE FROM old_userlist WHERE id = '$original_email_id'";
    $delete_original_email = $conn->query($sql);

// ############ PRINT email managed
    echo $domain_name_total_count." - ".$row_email_username."@".$row_email_domain."/".$row_password."\n";

    }

} else {
    echo "0 results";
}

//list results
echo "\n\n";
echo "Imported ".$domain_name_total_count." emails and ";
echo $password_total_count." passwords\n";
echo $username_new_count." new username entires of which ";
echo $username_duplicate_count." where duplicate.\n";
echo $domain_name_new_count." new domain entires of which ";
echo $domain_name_duplicate_count." where duplicate.\n";
echo $email_adress_new_count." new email address entires of which ";
echo $email_adress_duplicate_count." where duplicate.\n";
echo $password_new_count." new password entires of which ";
echo $password_duplicate_count." where duplicate.\n";
echo $relationship_user_source_new_count." new relationship entires (user/source) of which ";
echo $relationship_user_source_duplicate_count." where duplicate.\n";
echo $relationship_email_source_new_count." new relationship entires (email adress/source) of which ";
echo $relationship_email_source_duplicate_count." where duplicate.\n";
echo $relationship_password_source_new_count." new relationship entires (password/source) of which ";
echo $relationship_password_source_duplicate_count." where duplicate.\n";
echo $relationship_email_password_new_count." new relationship entires (email/password) of which ";
echo $relationship_email_password_duplicate_count." where duplicate.\n";

$conn->close();
\$\endgroup\$
3
  • \$\begingroup\$ Do you have indexes on your database columns? Not having the right indexes can completely torpedo performance. \$\endgroup\$
    – tsleyson
    May 27, 2017 at 17:26
  • 2
    \$\begingroup\$ You should explain in your question what you actually want to achieve. The whole approach seems a bit odd to me. Why would you want your usernames and passwords in different tables? And how exactly do you want to deal with duplicates? I added a general answer, but adding more information will give you better answers (alternatively, you may first want to apply my suggestions, and then ask a follow-up question with more information). \$\endgroup\$
    – tim
    May 27, 2017 at 20:18
  • \$\begingroup\$ mysql has a function called count.. select count(*) from table is faster than a select * and an incrementing php loop. In fact, even using PHP's count function is faster. \$\endgroup\$ Jun 19, 2017 at 16:40

3 Answers 3

3
\$\begingroup\$

Before working on performance, I would work on readability, code structure, and security. The first two - and surprisingly the third - can then help in later speeding up the code.

Security

You are vulnerable to second order SQL injection because you insert user-supplied data in your queries.

You need to use prepared statements to defend against this.

By using prepared statements, you can also prepare a query once, and then execute it multiple times with different data, thus speeding up the process.

Structure

Instead of structuring your code via comments, you should create functions. They will be easier to profile to find bottlenecks in your code, and you will get rid of some duplication.

It will also reduce nesting and make your code generally easier to read. This will make it a lot easier to see what is actually happening, and where there may be bottlenecks.

It will also get rid of your comments, as the function names will serve as documentation. This is especially helpful in your case, as your comments are sometimes wrong (such as saying that you are checking for the domain name when you are actually checking for the username, or that you are checking for the email when you are checking for the password).

These wrong comments likely resulted from copy-pasted code, which is always a sign that you should extract the code to a function which you can reuse.

Misc

  • It seems from your comments that you are checking for exact duplicates, but you are using like. This is slower than = and may result in errors.
  • When you encounter a duplicate, you fetch and iterate over all duplicate results to get the last id. This seems unstable and unnecessary.
\$\endgroup\$
2
\$\begingroup\$

First of all, I really question your database schema, as it has some some areas where it seems you have perhaps over-normalized your data (is there really a functional reason to have a separate domain and source tables?), some areas where so seem to be making decisions to de-normalize (why do you need to store username in multiple tables?), and some tables that don't seem to make sense at all (why is there an email table that doesn't store email address?).

Furthermore, it seems your tables have meaningless field names like e, p, u, s. What is the point of storing data in a data store if you can't even understand the data structure?


The primary performance problem you have is that you have fallen into the anti-pattern of querying in loops. It looks like what you are doing is some sort of data migration. The chances are that this whole thing can be done based on a few queries. For example, let's look at using INSERT ... SELECT syntax to simplify this operation. To do this, let's assume that the email address / password combination from the source table is the main thing you are concerned about here. And let's also assume that your relationships table is the primary table you are going to be migrating the data into

Let's start with this.

INSERT INTO relationships
(email, password, source)
SELECT email, password, 'old_userlist'
FROM old_userlist

One query populates your entire relationships table.

If you then want to populate some of your table which I noted above as potentially being "over-normalized" you can do it from this table

For example:

INSERT INTO domain (domain)
SELECT DISTINCT SUBSTRING_INDEX(email, '@', -1) AS domain
FROM relationships

INSERT INTO source (source)
SELECT DISTINCT source FROM relationships

etc.

This eliminates all need for looping.

This also eliminates need for row by row deletion from source table, you can just DROP or TRUNCATE the source table after your operations are done.

You may even consider performing this whole operation within a transaction if you want to only commit the change after the entire series of actions in the migration is complete.

By the way, I agree with the comment in other answers around prepared statements - you should always use these on those rare occasions where you do need to query in loops or when you are injecting variables into where, sort, order, etc. clauses - but, in this case, I am guessing you can get away from either of those circumstances.


You should get out of the habit of using SELECT *. Star selects:

  • obfuscate the date structure from reader of the code (what fields do I have available to work with?)
  • can cause unnecessary bandwidth to be consumed between DB and application. Why send all fields if you only need one or two?
  • can make your code fragile to table schema changes.

There is no reason whatsoever for you to be using LIKE in these queries, since you are doing exact matches.


You have some style problems with your code:

  • Inconsistent indentation (particularly with you while loop here, where it is difficult to see where it ends).
  • Unnecessary nesting. For example, if you simply invert your condition after the first query, you can de-nest most of the code in this script.

For example:

if ($result->num_rows === 0) {
    echo "0 results";
    exit;
}
// rest of code, now without nesting

Look at ways to minimize code nesting and code path. Inverting conditions, early exits, and removing unnecessary else conditions are all strategies in keeping your code nesting/branching to a minimum, meaning you have less complex code that is easier to read, maintain, and test.


To generate your insertion counts you should not need to keep manual counters. For example, if you used the INSERT ... SELECT strategy I mentioned above, $conn->affected_rows would give you the count for number of rows inserted/updated as a result of the last query execution.


You should strive to keep database user/password credentials out of your code, as this is a security problem (do you really want your entire password history stored in revision control?). Ideally, these should be derived from environmental configuration.

\$\endgroup\$
2
\$\begingroup\$

To answer the question, "Any tips on speeding it up?", I would say that you need to bear in mind all the disk access you're doing, and move that outside your main loop, similar to what Mike Brant said about "the anti-pattern of querying in loops." Also, keep in mind that writing to a database is more expensive (in time) than reading. You can move all of those INSERTs outside of your loop by storing the data you need to insert in a temporary array.

Visually, your script looks (something) like this:

Loop over rows:
  query(SELECT * FROM ... where something = $row['x']);
    if (something) query(INSERT ...); //expensive!
    ...
End Loop

You can change that to

Loop over rows:
  query(SELECT * FROM ... where something = $row['x'])
    if (something) $TMP_ARRAY[] = 'INSERT ...';
End Loop
Loop over $TMP_ARRAY
  if (something) big_query .= ($TMP_ARRAY[$i]); //build up big INSERT query
End Loop
query(big_query)

I would suggest at first just commenting out your INSERT queries and see how fast the script runs, then moving your INSERTS outside of your main loop (where you print out results) and seeing if that gets it to run any faster.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.