fuel / core

Fuel PHP Framework - The core of the Fuel v1 framework
http://fuelphp.com
811 stars 347 forks source link

Agent class is too heavy #1790

Open kenjis opened 10 years ago

kenjis commented 10 years ago

When I install Fuel 1.7.2 and I browse Welcome page.

Page rendered in 0.0739s using 0.661mb of memory.

And I add one line in Welcome controller.

    public function action_index()
    {
        echo Agent::browser(); // here
        return Response::forge(View::forge('welcome/index'));
    }

Then I browse Welcome page.

Page rendered in 3.5703s using 44.822mb of memory.

In fact, the initial access takes much more time and memory (about 120MB) to download browsercap file.

Is this a bug or not?

kenjis commented 10 years ago

I set browscap in php.ini and it reduced the memory usage drastically. But it took more or less the same time.

<?php

$start = microtime(true);

var_dump($_SERVER['HTTP_USER_AGENT']);
var_dump(get_browser());

$end = microtime(true);
$time = $end - $start;
var_dump($time);

It also took 3.5 sec. So it seems get_browser() is too slow.

WanWizard commented 10 years ago

That can't be helped.

If you have access to the php.ini, it's absolutely advised to use that, since the C++ code used to parse the browsecap file is much faster then doing it in PHP itself.

There are however situations (for example on shared hosts) where you don't have access to php.ini, and where no browsecap is setup. In that case, Agent allows you to do it in PHP.

You'll see here https://github.com/fuel/core/blob/1.8/develop/classes/agent.php#L195 that it will use get_browser() first, and it only falls back to fetching and parsing it itself when get_browser() fails.

kenjis commented 10 years ago

Even get_browser() is too slow now. It is because browscap.ini has been grown too big. https://groups.google.com/forum/#!topic/browscap/IAwzqzRSSWI

I can't wait more than 3 secs. I must think to use another implementation.

WanWizard commented 10 years ago

Difference is that get_browser() parses the browscap file on every request, Agent does it once, and optimizes the result for faster access.

Have you compared the two, so Agent when fetching and parsing the file, and Agent using the optimized cache file?

kenjis commented 10 years ago

In my understanding, without php.ini browscap config and with Fuel default config, Agent class uses the optimized cache file after fetched browscap file.

In my fuel app cache folder, there is browscap.cache (7.7MB).

So it resulted Page rendered in 3.5703s using 44.822mb of memory.

WanWizard commented 10 years ago

So it takes 3,5 seconds to do an optimized seek? That is indeed not good. Problem I think is that now it looks over the agent strings with a regex, I can imagine that this takes a while. Question is how to fix it...

kenjis commented 10 years ago

The first post was results on Mac OS X.

I run the same code with 1.8/develop on Linux (32bit).

Welcome page

Page rendered in 0.011s using 0.323mb of memory.

Added Agent::browser()

Default (php.ini: browscap => no value)

First time (fetch and parse browscap file)

Page rendered in 28.8346s using 71.589mb of memory.

Second time

Page rendered in 1.2651s using 29.342mb of memory.
Set browscap in php.ini
Page rendered in 0.854s using 0.426mb of memory.

It seems Linux is much faster than OS X.

kenjis commented 10 years ago

I run the same code with 1.8/develop on Mac OS X. Because 1.8/develop might be faster than 1.7.2.

Welcome page

Page rendered in 0.0093s using 0.438mb of memory.

Added Agent::browser()

Default (php.ini: browscap => no value)

First time (fetch and parse browscap file)

Page rendered in 23.3956s using 112.952mb of memory.

Second time

Page rendered in 3.6668s using 44.826mb of memory.
Set browscap in php.ini
Page rendered in 3.5592s using 0.459mb of memory.

Again, Mac OS X is too slow.

kenjis commented 10 years ago

I wrote the code which uses https://github.com/browscap/browscap-php in stead of get_browser() in Agent class.

            $cacheDir = APPPATH.'cache/fuel/agent';
            $browscap = new \phpbrowscap\Browscap($cacheDir);
            $browscap->remoteIniUrl = 'http://browscap.org/stream?q=Lite_PHP_BrowsCapINI';
            $browscap->doAutoUpdate = false;
            $browser = $browscap->getBrowser(static::$user_agent, true);

And I got below on Mac OS X.

Page rendered in 0.0513s using 6.456mb of memory.
WanWizard commented 10 years ago

Hmm, have to look at that, because Agent is based on exactly the same parser code (they are both forks of GaretJax). You're sure you used the Lite version of the browscap file in all tests?

kenjis commented 10 years ago

@WanWizard

Yes.

$browscap->remoteIniUrl = 'http://browscap.org/stream?q=Lite_PHP_BrowsCapINI';

$ ls -l
total 46232
-rw-rw-rw-  1 kenji  staff  8033080 10 30 21:25 browscap.cache
-rw-r--r--  1 kenji  staff  6043246 10 31 06:20 browscap.ini
-rw-rw-rw-  1 kenji  staff  6043381 10 30 21:25 browscap_file.cache
-rw-r--r--  1 kenji  staff  3540744 10 31 06:20 cache.php
WanWizard commented 10 years ago

I got that, I was asking about the previous tests (with php.ini and Agent), there is a huge difference between the standard file and the lite file.

kenjis commented 10 years ago

I did not change Fuel Agent config, and I downloaded Lite version for php.ini. They are surely all Lite version.

WanWizard commented 10 years ago

Ok. Our fork is now over 3 years old. Guess the code needs to be looked at then, to see what was improved.

it-can commented 9 years ago

This also looks like a nice package: https://github.com/crossjoin/Browscap

WanWizard commented 9 years ago

Looks promising...

julesjanssen commented 9 years ago

Not exactly the same, but I use https://github.com/piwik/device-detector Combine that package with an extended Agent class and you can use it as before.

WanWizard commented 9 years ago

In the forum https://github.com/mcmillan/special_agent is suggested, which would solve the issue if you're only interested in mobile device information.

emlynwest commented 9 years ago

Perhaps there can be two halves to this? One of the previously suggested solutions for everything then a lighter version for if you are interested in mobile only.

kenjis commented 9 years ago

Crossjoin\Browscap is the best replacement to me.

I wrote ad hoc patch to use Crossjoin\Browscap: http://blog.a-way-out.net/blog/2015/02/26/fuelphp-agent-class-heavy/ (sorry the article written in Japanese)

It seems browscap.ini is getting bigger and bigger, so the memory usage in updating the file may be a big problem.