too many CoreModule inits?

valiant

Joined: 2003-01-04
Posts: 32509

Posted: Mon, 2004-01-05 22:43

why do you instantiate class CoreModule 2 times?
in GalleryInitFirstPass() for bootstrap()
and
in GalleryInitSecondPass() (
loadmodule('core') which does init CoreModule
and init() it again on line 185 "$ret = $coreModule->init();"
)
isn't that a init() too much?

bharat

Joined: 2002-05-21
Posts: 7994

Posted: Mon, 2004-01-05 23:03

I'm pretty sure that we don't actually call core's init() more than once. The core's bootstrap() method is different and special. Basically, in order for us to successfully initialize modules we need to have enough of the framework loaded so even though core itself is a module we cannot initialize it until it has been at least partially loaded itself. So we have a special bootstrap() method that gets enough of the system running before loading the core module as a regular module.

Since we do so much in core's bootstrap() we don't need to do all that much in core's init() but there are still a few odd tasks that it's useful to do at that point. I should probably move some of the extra code from bootstrap() into init() just for parity with other modules.

valiant

Joined: 2003-01-04
Posts: 32509

Posted: Tue, 2004-01-06 07:35

well, thanks for the description of bootstrap(), but that's not exactly i was talking about.
in GalleryInitSecondPass() you call $gallery->init() twice.
once in line 135 "list ($ret, $coreModule) = $gallery->loadModule('core');"
and again in line 185 "$ret = $coreModule->init();"

and in between the 2 init() you don't change any config/data which is set by init().

bharat

Joined: 2002-05-21
Posts: 7994

Posted: Tue, 2004-01-06 10:09

I admit that is confusing code, but it's not actually initializing the core module twice. During the core bootstrap process, it calls:

GalleryPluginMap::setPluginInstance('module', 'core', $this);

which updates the GalleryPluginMap's cache so that when GalleryPluginMap::loadPlugin() is called on the core module, it gets the module from the cache and doesn't initialize it a second time. Correct me if I'm wrong...

valiant

Joined: 2003-01-04
Posts: 32509

Posted: Tue, 2004-01-06 11:58

sorry, you're right. i overlooked exactly this code line
thanks - andy

ps: do you have a rough count of sql queries per request? and do you plan caching queries?

valiant

Joined: 2003-01-04
Posts: 32509

Posted: Tue, 2004-01-06 22:34

ok, the question @ caching remains, i'll answer myself the other question about nr. of sql queries / session.
after investigating the whole process of a simple downloaditem request (boy that took time ;) ) i came with the following:

GalleryInitFirstPass()
function_exists() for 5 functions which are supported since php 4.2 or similar.
why do this for each request again and again?why not only in a setup / config mode?

GalleryInitSecondPass()
_SQL queries: Total 4
1 (check version and load core data) in $coreModule->install() -> loadPlugin 'core'
1 (load session data) in initSession() -> GallerySession->init()->_loadSessionData()
1 (load user data $activeUser) $gallery->initTranslator() -> loadEntitiesById()
1 (Load the module list) in fetchPluginList()

guess you could load session and user data at once...

--------------------------------------------------------------
end init
begin url specific
--------------------------------------------------------------
example:
main.php?g2_view=core:DownloadItem&g2_itemId=9
_SQL querires: Total 2
1 (get item data) in renderImmediate()
1 (get permission) in renderImmediate()
_time: readfile()

that makes a total of 6 sql queries and "some" php code ;)
just do never output buffering when serving video files with G2 ;) (ever thought of 40mb memory / reques)

bharat

Joined: 2002-05-21
Posts: 7994

Posted: Tue, 2004-01-06 23:09

The first rule of optimizing is to profile your code and then analyze the results to figure out what you should be optimizing.

Turn on the profiling option in config.php and you'll get profiling output at the bottom of every page. Every SQL call goes is recorded as modules.core.classes.Gallery::search so you can see the total number of calls, the average time per call and the total time taken.

In my test environments, even if I have a lot of queries they take a very small percentage of the overall time spent handling the request. I think that it's still to early to do a real optimization pass, but my guess is that a lot of the time is spent in the page rendering code, which is going to get rewritten anyway. Combining multiple queries into one is the kind of thing that makes the code more confusing for a very very tiny performance gain.

There's a caching infrastructure already in place (and in use) to cache expensive lookups. If we find queries that are being executed multiple times that are slow (I don't know of any, at the moment) it is trivial to cache them. See GalleryDataCache and how it's used.

If you're really interested in working on this, it would be very useful it you could start to identify some of the hot spots. Turn on the profiling code and then see if you can find places where we are taking an unreasonable amount of time to do processing. Take the "function_exists" call for example. How expensive is it? Write a script that times 1,000,000 calls to function exists and calculates the time spent per call. Is it significant? What would be the performance increase by caching this value instead?

valiant

Joined: 2003-01-04
Posts: 32509

Posted: Tue, 2004-01-06 23:27

actually, i wanted to get to know the application and thought, why not summarize resource intensiv tasks while going through all lines of code.
of course you're right about everything, but
@caching: yeah, i like the caching during the request, but what about caching which lives longer than a request? all default request do the same things. and caching for a user session.
i'll definatly do what you proposed (profiling), i think G2 is quite CPU intensive when serving a page with 12 images to several users at the same time.

bharat

Joined: 2002-05-21
Posts: 7994

Posted: Wed, 2004-01-07 00:45

I didn't mean to criticize you for going down this path; sorry that my post came off that way. I just mean to say that my strategy is to spend more time profiling and less time optimizing until we figure out exactly what's slow. And it seems like you're on that path already so all's well

I agree that we can probably add caching that persists longer than the lifetime of the request. I haven't spent a lot of time thinking about it but I'm sure that there are going to be at least a few things that we can handle that way and the database is an easy place to store and retrieve that stuff.

valiant Joined: 2003-01-04 Posts: 32509	Posted: Mon, 2004-01-05 22:43
	why do you instantiate class CoreModule 2 times? in GalleryInitFirstPass() for bootstrap() and in GalleryInitSecondPass() ( loadmodule('core') which does init CoreModule and init() it again on line 185 "$ret = $coreModule->init();" ) isn't that a init() too much?
	XUser login Username: * Password: * Create new account Request new password
bharat Joined: 2002-05-21 Posts: 7994	Posted: Mon, 2004-01-05 23:03
	I'm pretty sure that we don't actually call core's init() more than once. The core's bootstrap() method is different and special. Basically, in order for us to successfully initialize modules we need to have enough of the framework loaded so even though core itself is a module we cannot initialize it until it has been at least partially loaded itself. So we have a special bootstrap() method that gets enough of the system running before loading the core module as a regular module. Since we do so much in core's bootstrap() we don't need to do all that much in core's init() but there are still a few odd tasks that it's useful to do at that point. I should probably move some of the extra code from bootstrap() into init() just for parity with other modules.

valiant Joined: 2003-01-04 Posts: 32509	Posted: Tue, 2004-01-06 07:35
	well, thanks for the description of bootstrap(), but that's not exactly i was talking about. in GalleryInitSecondPass() you call $gallery->init() twice. once in line 135 "list ($ret, $coreModule) = $gallery->loadModule('core');" and again in line 185 "$ret = $coreModule->init();" and in between the 2 init() you don't change any config/data which is set by init().

bharat Joined: 2002-05-21 Posts: 7994	Posted: Tue, 2004-01-06 10:09
	I admit that is confusing code, but it's not actually initializing the core module twice. During the core bootstrap process, it calls: `GalleryPluginMap::setPluginInstance('module', 'core', $this);` which updates the GalleryPluginMap's cache so that when GalleryPluginMap::loadPlugin() is called on the core module, it gets the module from the cache and doesn't initialize it a second time. Correct me if I'm wrong...

valiant Joined: 2003-01-04 Posts: 32509	Posted: Tue, 2004-01-06 11:58
	sorry, you're right. i overlooked exactly this code line thanks - andy ps: do you have a rough count of sql queries per request? and do you plan caching queries?

valiant Joined: 2003-01-04 Posts: 32509	Posted: Tue, 2004-01-06 22:34
	ok, the question @ caching remains, i'll answer myself the other question about nr. of sql queries / session. after investigating the whole process of a simple downloaditem request (boy that took time ;) ) i came with the following: GalleryInitFirstPass() function_exists() for 5 functions which are supported since php 4.2 or similar. why do this for each request again and again?why not only in a setup / config mode? GalleryInitSecondPass() _SQL queries: Total 4 1 (check version and load core data) in $coreModule->install() -> loadPlugin 'core' 1 (load session data) in initSession() -> GallerySession->init()->_loadSessionData() 1 (load user data $activeUser) $gallery->initTranslator() -> loadEntitiesById() 1 (Load the module list) in fetchPluginList() guess you could load session and user data at once... -------------------------------------------------------------- end init begin url specific -------------------------------------------------------------- example: main.php?g2_view=core:DownloadItem&g2_itemId=9 _SQL querires: Total 2 1 (get item data) in renderImmediate() 1 (get permission) in renderImmediate() _time: readfile() that makes a total of 6 sql queries and "some" php code ;) just do never output buffering when serving video files with G2 ;) (ever thought of 40mb memory / reques)

bharat Joined: 2002-05-21 Posts: 7994	Posted: Tue, 2004-01-06 23:09
	The first rule of optimizing is to profile your code and then analyze the results to figure out what you should be optimizing. Turn on the profiling option in config.php and you'll get profiling output at the bottom of every page. Every SQL call goes is recorded as modules.core.classes.Gallery::search so you can see the total number of calls, the average time per call and the total time taken. In my test environments, even if I have a lot of queries they take a very small percentage of the overall time spent handling the request. I think that it's still to early to do a real optimization pass, but my guess is that a lot of the time is spent in the page rendering code, which is going to get rewritten anyway. Combining multiple queries into one is the kind of thing that makes the code more confusing for a very very tiny performance gain. There's a caching infrastructure already in place (and in use) to cache expensive lookups. If we find queries that are being executed multiple times that are slow (I don't know of any, at the moment) it is trivial to cache them. See GalleryDataCache and how it's used. If you're really interested in working on this, it would be very useful it you could start to identify some of the hot spots. Turn on the profiling code and then see if you can find places where we are taking an unreasonable amount of time to do processing. Take the "function_exists" call for example. How expensive is it? Write a script that times 1,000,000 calls to function exists and calculates the time spent per call. Is it significant? What would be the performance increase by caching this value instead?

valiant Joined: 2003-01-04 Posts: 32509	Posted: Tue, 2004-01-06 23:27
	actually, i wanted to get to know the application and thought, why not summarize resource intensiv tasks while going through all lines of code. of course you're right about everything, but @caching: yeah, i like the caching during the request, but what about caching which lives longer than a request? all default request do the same things. and caching for a user session. i'll definatly do what you proposed (profiling), i think G2 is quite CPU intensive when serving a page with 12 images to several users at the same time.

bharat Joined: 2002-05-21 Posts: 7994	Posted: Wed, 2004-01-07 00:45
	I didn't mean to criticize you for going down this path; sorry that my post came off that way. I just mean to say that my strategy is to spend more time profiling and less time optimizing until we figure out exactly what's slow. And it seems like you're on that path already so all's well I agree that we can probably add caching that persists longer than the lifetime of the request. I haven't spent a lot of time thinking about it but I'm sure that there are going to be at least a few things that we can handle that way and the database is an easy place to store and retrieve that stuff.

too many CoreModule inits?

XUser login