Optimize inserting keywords into tl_search_index

[leofeyer]

Using a combined insert query to add the keywords to tl_search_index will greatly improve the performance.

/** @var Connection $objConnection */
$objConnection = \System::getContainer()->get('database_connection');

// Remove existing index
$objDatabase->prepare("DELETE FROM tl_search_index WHERE pid=?")
			->execute($intInsertId);

$arrValues = array();

foreach ($arrIndex as $k=>$v)
{
	$arrValues[] = sprintf('(%s, %s, %s, %s)', (int) $intInsertId, $objConnection->quote($k), $objConnection->quote($v), $objConnection->quote($arrData['language']));
}

// Create the new index
$objDatabase->query("INSERT INTO tl_search_index (pid, word, relevance, language) VALUES " . implode(', ', $arrValues));

I was able to rebuild the search index on contao.org in about 2 minutes compared to over 10 minutes before.

@contao/developers Shall we change this in Contao 4.4 as well?

[aschempp]

Are you sure about the database quoting? Also, why mix the two connections and not just use Doctrine?

[aschempp]

On a similar note, this can be done just with the legacy class.

// Remove existing index
$objDatabase->prepare("DELETE FROM tl_search_index WHERE pid=?")
			->execute($intInsertId);

$arrQuery = array();
$arrValues = array();

foreach ($arrIndex as $k=>$v)
{
        $arrQuery[] = '(%s, %s, %s, %s)';
	$arrValues[] = (int) $intInsertId;
	$arrValues[] = $k;
	$arrValues[] = $v;
	$arrValues[] = $arrData['language'];
}

// Create the new index
$objDatabase->prepare("INSERT INTO tl_search_index (pid, word, relevance, language) VALUES " . implode(', ', $arrQuery))->execute($arrValues);

[leofeyer]

I am sure that we should quote the values if that is what you mean. 😄

We might as well use only Doctrine of course. Or add a quote() method to our Database class. Those implementation details need to be discussed.

The code was just a quick fix for contao.org.

[leofeyer]

I don't like your second solution. It works for sure, but we are building two possibly giant arrays that consume a lot of memory for nothing.

[aschempp]

Really? Counting the characters, I think ('foo', 'bar') is exactly the same amount as (%s, %s) and foo, bar? 😁

[leofeyer]

Except your approach requires a lot of vsprintf() to build the query string.

[aschempp]

It requires one, instead of multiple sprintf?

[leofeyer]

Sorry, I meant it requires a giant vsprintf() instead of multiple small sprintf(). No idea if there is any performance difference though.

[m-vo]

If the sprintf() / vsprintf() performance is the bottleneck then plain old string concatenation would probably be the way to go. :-)

(I'd be more concerned about possible injections. )

[leofeyer]

It seems that one giant vsprintf() is more performant: https://3v4l.org/Up3jQ

[leofeyer]

Implemented in 6f8a020.

[m-vo]

First of all: I'm fine with the changes as this really makes things faster. :D

And I really don't want to nitpick but rather raise awareness that those tests should probably better be profiled inside the application. If you for instance swap the execution order of the two tested versions (https://3v4l.org/lvnqS) the already small differences (overall same magnittude) gets even smaller showing that these tests are in no way free of side effects.