document.parser.inc.php code size

If multiple includes can slow performance, then please review in detail my spreadsheet Raymond. There’s quite a few (very tiny!) files we could consolidate elsewhere.

Ehm, included files indeed depend on disk IO, but fortunately, Linux handles this very nicely.
When a file is frequently requested from hard disk, it keeps it buffered in memory, so no need to worry about that.

I agree that having lots of includes doesn’t make sense either.
Having a number of well thought out classes could do the trick.

For example, if a page is cached, and no snippets needs evaluating, there is no need to load all kind of API’s which could be accesed from within snippets.
If a page is cached but there is need to process snippets, load the processTags object.
Everything related to processing Tags (and thus snippets an API calls) can be put into that objects class.
If a page is not cached, load the createDocument object, which has an instance of processTags and so on

Anyway, I tried to strip down the parser to the bare minimum, not sure if it can be made even smaller, but it’s now about 800 lines of code.
Note that it only is able to serve cached pages in this form.

But the numbers speak for themselfs: (PIII 800)

Before: 35 req/sec (Code size ~2500 lines)
After : 46 req/sec (Code size ~800 lines)
(With invokeEvent commented out and ~800 lines of code: 57 req/sec)

The Plugin events etc do add some code and degradation of performance, but someone is working on that?

But as you see, for cached pages, the code size significantly adds to the overhead.

Remon

EDIT:
The results aren’t consistent on multiple runs, but I’m testing on my own machine now, so... At least, it’s faster with smaller parser ;-) )

EDIT 2:
Attached stripped down parser class

Remon, if you strip out the API calls and so forth making for a smaller code parsing base, where are all the API calls going?

Not sure about Remon’s thoughts on this subject, but I’d say in a separate class file that can be loaded by the parser, or theoretically by any other PHP code -- this could potentially be useful for integration with existing PHP systems that need access to MODx functionality or data.

From an OOP point of view, each "problem" should be a seperate class.
So a class for database access, a class for creating the document, maybe even a seperate class for the processTags function, but php seems not to be designed for OOP (they told me that ;-) ) so too much classes and thus includes could probably harm performance instead of improving things.

About the "bare minimum" parser, which I called Tatoo.

The idea is to go for the common case.
Completely cached files should be the common case (well, let’s assume that).

Everything related to the document ((Web) user access, Plugin cache etc etc) is all in the cached file.
So, in theory no need for database accesss (now, there seems to be always database access for the Plugin system, but also in the checkCache() function :-( )

So, everything related to database is put into it’s own class.

The mini parser of course should be able to access the database if needed (for example, to log an event) so I suggest to load the database object on demand.
Like this:



This way the minimal parser can be kept really small, and if some functionality is needed, it’s loaded on demand...

What do you think?
Any pointers on possible classes?

Remon

Some suggested class/file names

class TattooParser --> tattoo.parser.class.inc.php
class TattooAPI --> tattoo.api.class.inc.php

I think this is moving exactly where we need to go; and I’ll mention that the idea of the common case of all cached files may help us get a solution where if database access goes down, the site can still be running, defaulting back to the cache always if the database in inaccessible for any reason.

Quote from: OpenGeek at Nov 16, 2005, 01:39 PM

class TattooParser --> tattoo.parser.class.inc.php
class TattooAPI --> tattoo.api.class.inc.php

I’m probably being way too picky here, but could we move away from the whole modx.parser.class.dynamic.kitchensink.inc.php thing? I just don’t see the point. why not parser.php or api.php? Even tattoo.parser.php or tattoo.api.php would be fine with me I just don’t see the reason for all of the class, inc, dynamic, static, processor stuff in the filename. If there’s a reason for having the filename the way they are that’s fine, but there’s no reason to complicate things if it’s not necessary. Especially, since it’s usually just repeating what the folder structure has alread told you.

Plus it’s more Etomitishness we can leave in the past where it belongs

Quote from: aNoble at Nov 16, 2005, 03:14 PM

Quote from: OpenGeek at Nov 16, 2005, 01:39 PM

class TattooParser --> tattoo.parser.class.inc.php
class TattooAPI --> tattoo.api.class.inc.php

I’m probably being way too picky here, but could we move away from the whole modx.parser.class.dynamic.kitchensink.inc.php thing? I just don’t see the point. why not parser.php or api.php? Even tattoo.parser.php or tattoo.api.php would be fine with me I just don’t see the reason for all of the class, inc, dynamic, static, processor stuff in the filename. If there’s a reason for having the filename the way they are that’s fine, but there’s no reason to complicate things if it’s not necessary. Especially, since it’s usually just repeating what the folder structure has alread told you.

Plus it’s more Etomitishness we can leave in the past where it belongs

LOL !

I vote for YES

Remon

Quote from: aNoble at Nov 16, 2005, 03:14 PM

I’m probably being way too picky here, but could we move away from the whole modx.parser.class.dynamic.kitchensink.inc.php thing? I just don’t see the point. why not parser.php or api.php? Even tattoo.parser.php or tattoo.api.php would be fine with me I just don’t see the reason for all of the class, inc, dynamic, static, processor stuff in the filename. If there’s a reason for having the filename the way they are that’s fine, but there’s no reason to complicate things if it’s not necessary. Especially, since it’s usually just repeating what the folder structure has alread told you.

I’m in agreement here. And am also for leaving off the tattoo part also. I will hazard a guess that they originally put the kitch.sink.desciption.inc.php in because of the error class didn’t output the path names and it probably helped them figure out what was breaking and where. But the way the errors are handled is another topic entirely... another area we can improve upon.

Plus it’s more Etomitishness we can leave in the past where it belongs

Amen!

I’ll thrown in that I personally like the notation of specifying if a PHP file is an include (.inc), a class (.class), etc. -- we definitely don’t need the tattoo, that’s just redundant, but the notation for identifying what role the PHP file plays is important IMO. There are too many ways to use PHP files not to help identify the type of PHP file it is with a naming convention. And I don’t think this is anything specific to Etomite, just a well adopted coding style.