bcosca / fatfree

A powerful yet easy-to-use PHP micro-framework designed to help you build dynamic and robust Web applications - fast!
2.66k stars 446 forks source link

Memory usage trough the roof with simple query #1143

Closed leancode closed 5 years ago

leancode commented 5 years ago

I am having the following select: $stores = $store_view->select('store_no', ["store_no > ?'')",0]); It results in a fairly reasonable result set: 2942 records. I am using select here instead of load because the view has 100s of columns and I don't need that data. I simply need the store numbers at this point. I am receiving:

PHP Fatal error:  Allowed memory size of 536870912 bytes exhausted (tried to allocate 72 bytes) in /home/test/vendor/bcosca/fatfree/lib/db/sql/mapper.php on line 593
Fatal error: Allowed memory size of 536870912 bytes exhausted (tried to allocate 72 bytes)
[vendor/bcosca/fatfree/lib/db/sql/mapper.php:593]

This is quite unexpected and I would say a bug. 512mb should be a reasonable amount of memory to return such a result set. I presume that what happens is that it executes the select like a "load" trying to load all 100s of columns instead of just loading the one.

pauljherring commented 5 years ago

I am using select here instead of load because the view has 100s of columns and I don't need that data.

So don't include the columns? Just pull back the ones you actually need.

$store_view = new \DB\SQL\Mapper($db, 'stores', 'store_id,name,location,...', 0);

Could probably go back to using load with that.

That said, it sounds like your DB could do with some normalisation.. :smile:

ikkez commented 5 years ago

I made a quick test locally (F3 v3.6.5, php7.2, mysql 5.6).. I had no problems loading 5000 records incl. 40 columns with a memory_limit at 128M using the DB\SQL\Mapper or Cortex.. peak was at ~86mb once or even below.. I was fetching it from a table instead of a view though, but that shouldnt make a difference for the mapper.

pauljherring commented 5 years ago

incl. 40 columns

From the OP, I think that should be closer to 400...

ikkez commented 5 years ago

Nope, from his sample, he tries to load only 1 column and hit the mark. Any executed query can be viewed with $db->log.. I'd suggest to add a limit clause first, and see the sql log if the query is right @leancode

pauljherring commented 5 years ago

Ah, I didn't have sight of any sample.

leancode commented 5 years ago

First of all thanks for all the feedback, I am really happy to see this activity around such an awesome framework.

Bit more info here to the problem at hand. I have updated the full query which is used just in case it has anything to do with the issue internally. I had simplified it earlier.

With a limit the query runs perfectly well:

$storesdb = new DB('mysql:host='.MY_HOST.';dbname=storesdb',MY_USER,MY_PASS); $store_view = new Mapper($storesdb,'store_view'); $stores = $store_view->select('store_no', ["store_no > ? AND (na IS NULL OR na = '')",0],['limit'=>100]); echo round(memory_get_peak_usage(true)/1024/1024,2)." MB\n"; echo $storesdb->log(); echo count($stores)." records fetched\n"; $columns = $storesdb->exec(["SELECT count(*) FROM information_schema.columns WHERE table_name = 'store_view'"]); echo $columns[0]['count(*)']. " columns in table store_view\n";

Result:

25.00MB (3.5ms) SHOW columns FROM storesdb.store_view (0.8ms) SELECT storeno FROM store_view WHERE storeno > 0 AND (na IS NULL OR na = '') LIMIT 100 100 records fetched 191 columns in table store_view

For comparison I just upped my memory limit so it no longer runs out and ran the same query without limit. Result:

518.5 MB (3.5ms) SHOW columns FROM storesdb.store_view (3.7ms) SELECT storeno FROM store_view WHERE storeno > 0 AND (na IS NULL OR na = '')" 2168 records fetched 191 columns in table store_view

The interesting thing is that just recently 100 more columns were added and prior to that the query was running fine. So I know it's just the adding of columns that is causing the issue.

I meanwhile rewrote this query as a simple exec and its working consuming much less memory:

$stores = $storesdb->exec(["SELECT storeno FROM store_view WHERE storeno > 0 AND (na IS NULL OR na = '')"]);

Result:

4.5 MB (3.1ms) SHOW columns FROM storesdb.store_view (2.1ms) SELECT storeno FROM store_view WHERE storeno > 0 AND (na IS NULL OR na = '')" 2168 records fetched 191 columns in table store_view

So, finally I still think there is a bug in the ORM somewhere because there should not be such a tremendous difference in running the same query as a ->select() using the ORM or as a straight ->exec(). It should not consume all this memory on a select query like this. Its basically 4.5 MB vs 518 MB.

Anyone with any ideas?

ikkez commented 5 years ago

Well the difference is that you're creating 2168x an object of your Mapper class. Is that the raw \DB\SQL\Mapper class or did you have extended it or do any magic there? The $sql->log() shows that it is actually running the same query.. so the query or the DB is not to blame. As @pauljherring has suggested, I'd try to limit the fields in the mapper instantiation directly: new \DB\SQL\Mapper($db, 'store_view', 'storeno'); If you're not doing that, the mapper is, regardless of the defined fields in your select(), at least creating an empty fields map it knows from the db schema query (SHOW columns FROM storesdb.store_view), which means it's only populating storeno, but all other fields are still present but NULL. (though it could be that the schema query doesnt return all fields from a view, I'm not fully sure at this point). So that could maybe eat memory as well.

leancode commented 5 years ago

No the mapper class is not extended. Its just the raw mapper \DB\SQL\Mapper. I see your point about limiting this in the mapper. Good idea and had not thought about that. Still the difference in memory consumption is quite significant.

Just thought of something because of what you said. I am still running this on PHP 5.6. Could it be that the problem is with PHP internals storing objects in a really inefficient way in 5.6?

I will test it against 7.0 + and report back here. If its a PHP issue we can close this.

bcosca commented 5 years ago

You really should be using the third argument of the Mapper constructor else you will be having memory issues on tables with a large number of columns.

leancode commented 5 years ago

Thanks bcosca, I will close this for now, when I have a chance to test on php 7+ I will reopen if needed. Thanks for everyones responses