0

I have a code which fetches data from external API and commits it to DB afterwards:

protected function saveWidgetsToDatabase($widgetsDaily, Boost $boost, $date)
{
    echo "Saving widgets to DB... ";

    $widgets = Widget::all();
    foreach ($widgetsDaily as $widgetDaily) {
        $existingWidget = $widgets
            ->where('widget_id', $widgetDaily->id)
            ->where('date', $date)
            ->first();

        if ($existingWidget === null)
            $boost->widgets()->save(new Widget([
               ...
            ]));
        else
            $existingWidget->update([
                ...
            ]);
    }
}

Relation I have is that one Boost has many Widgets. Now, the issue I'm facing is bottleneck DB saving/updating as I need to update a widget only if it has same date and ID, otherwise I need to create new one.

We are talking about few thousands of records, so I believe that where clauses are pretty intensive.

I wanted to make a batch save, though I didn't quite make it.

Are there any chances of making this faster?

4
  • Do you have widget_id and date indexed? Commented Oct 23, 2017 at 18:30
  • No, not really...would that help? Commented Oct 23, 2017 at 18:34
  • Yes. Indexing helps the database find information faster, and would speed up your where queries. Commented Oct 23, 2017 at 18:36
  • Please provide the generated SQL. Commented Oct 25, 2017 at 14:34

1 Answer 1

1

When you call Widget::all();, that gets every single widget record in your database and creates a Widget instance for it. Therefore, $widgets will be a Collection of every Widget object stored in the database. If you have 10000 widget records, you'll have a Collection of 10000 Widget objects. This is obviously not what you want.

That also means that when you call $widgets->where()..., you're calling where() on the Collection object, which is using PHP to filter through the collection of objects, instead of using SQL to filter the database results.

There are a couple things you can do.

First, you know you only care about those widgets that have an id in the list of $widgetsDaily. So, limit your Widget query to only include those records that have a widget_id in that list of ids.

Second, add the date lookup to the database query as well.

Third, key the resulting collection by the widget_id field, so that you can directly access the item by the widget_id without having to loop through the entire collection looking for it every time.

protected function saveWidgetsToDatabase($widgetsDaily, Boost $boost, $date)
{
    // Get the only widget_ids we care about (assumes $widgetsDaily is a collection)
    $ids = $widgetsDaily->pluck('id')->all();

    // Get the target widgets from the database. This collection will only
    // contain widgets that we actually care about.
    $widgets = Widget::whereIn('widget_id', $ids)
        ->where('date', $date)
        ->get()
        ->keyBy('widget_id'); // rekey the resulting collection

    foreach ($widgetsDaily as $widgetDaily) {
        // Because the collection was rekeyed on widget_id, you can use
        // get(id) instead of having to use where('widget_id', id)->first()
        $existingWidget = $widgets->get($widgetDaily->id);

        if ($existingWidget === null)
            $boost->widgets()->save(new Widget([
               ...
            ]));
        else
            $existingWidget->update([
                ...
            ]);
    }
}
Sign up to request clarification or add additional context in comments.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.