Autoloader Usage / Removal of Manifest / Manifest Builder

87 views
Skip to first unread message

Christopher

unread,
Mar 2, 2011, 2:20:31 PM3/2/11
to SilverStripe Core Development
While this may require some extensive code reorganization, moving
towards a proper usage of PHP's autoloader would be a great thing to
have in SS 3.0.

As per the most common pattern, this would mean having classnames
reflected in the path/directory tree.

As an example, we currently have: /sapphire/core/model/SiteTree.php
with a class declaration of "class SiteTree".
Any instantiations of "SiteTree" rely on the manifest file, which
"includes" all PHP files available in the entire project path tree.

This would be changed to:

/Sapphire/Core/Model/SiteTree.php with a class declaration of "class
Sapphire_Core_Model_SiteTree".

An instantiation of this: $page = new Sapphire_Core_Model_SiteTree()"
would hit PHP's autoloader (and the registered SPL function) would
include the proper file based on the path represented in the class
name.

This negates the need for a manifest file (and manifest builder) -
also allowing for only *needed* PHP files to be included on page load
(rather than everything represented by the manifest).
In addition, it enforces better code organization.

Simon J Welsh

unread,
Mar 2, 2011, 2:33:13 PM3/2/11
to silverst...@googlegroups.com
Firstly, the manifest only includes every _config.php, not every PHP file. It only loads files as they're needed already. Try something like var_dump(class_exists('CliController', false)); (The false skips the call to the autoload functions)

Secondly, this will break everyone's code. Not just most people's, but everyone's.

Thirdly, it looks ugly. Can you imagine Sapphire_Core_Model_DataObject::get('Sapphire_Core_Model_SiteTree') everywhere?

While ManifestBuilder can always be improved, I'm against removing it, especially if it just gets replaced with something that, in my opinion, just looks ugly. Not to mention that once the manifest is built, getting the right path is a simple array lookup (hash tables make it really fast) rather than multiple string operations and file system checks.

> --
> You received this message because you are subscribed to the Google Groups "SilverStripe Core Development" group.
> To post to this group, send email to silverst...@googlegroups.com.
> To unsubscribe from this group, send email to silverstripe-d...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/silverstripe-dev?hl=en.
>

---
Simon Welsh
Admin of http://simon.geek.nz/

Who said Microsoft never created a bug-free program? The blue screen never, ever crashes!

http://www.thinkgeek.com/brain/gimme.cgi?wid=81d520e5e

Sam Minnée

unread,
Mar 2, 2011, 2:36:33 PM3/2/11
to silverst...@googlegroups.com
Yea, I'm with Simon on this one.

Christopher

unread,
Mar 2, 2011, 2:50:05 PM3/2/11
to SilverStripe Core Development
Thanks for the clarification, you are correct - the manifest file only
does a "require_once" on config files.

However, it should be mentioned that the manifest files are upwards of
300 kilobytes which is included on every page request. And obviously
must be regenerated (with ?flush) everytime you add or rename files.

As for the ugliness of a path-based autoloader, as it pertains to how
SS is structured and operates now, I definitely agree.

For other examples, have a look at Zend Framework, Magento, or
anything else that operates in that manner. There is a strong
promotion of code organization.
Magento in particular get's around the ugliness by implementing a
static getter for models. Eg. Mage::getModel( 'importexport' ).

As you said, this would break backwards compatibility, so there likely
wouldn't be any major revisions to how this operates.

I think the crux of why I started this post was specifically around
proper organization of classes (controllers, models, etc.), but that
would be another topic.
> > For more options, visit this group athttp://groups.google.com/group/silverstripe-dev?hl=en.
>
> ---
> Simon Welsh
> Admin ofhttp://simon.geek.nz/

Marcus Nyeholt

unread,
Mar 2, 2011, 5:42:22 PM3/2/11
to silverst...@googlegroups.com
A bytecode cache greatly reduces the hit of loading the manifest every page load. Sure, it's not available anywhere, but any site that would feel the pain of it loading should probably be running it. 

Marcus

Sam Minnée

unread,
Mar 2, 2011, 5:57:45 PM3/2/11
to silverst...@googlegroups.com
- In situations that don't have a bytecode cache, AJShort has been working on something that uses a PHP serialize() result, which is almost as fast as bytecode-cached PHP.
- Most of the bulk of the manifest is actually the map of the descendants of every class, not the classname -> filename map.

Hamish Campbell

unread,
Mar 2, 2011, 7:14:00 PM3/2/11
to SilverStripe Core Development
On Mar 3, 11:42 am, Marcus Nyeholt <nyeh...@gmail.com> wrote:
> ...any site that would feel the pain of it loading should probably be running it...

Or SilverStripe, for that matter =D

Christopher, I think this is only going to fly if you can show that
there is some sort of significant performance improvement that can be
made here without getting in the way of developers.

For example, requiring a particular folder structure for your module
code to enable auto-loading is just another thing to go wrong for
very little appreciable performance benefit.

Hamish

Sam Minnee

unread,
Mar 2, 2011, 8:03:52 PM3/2/11
to SilverStripe Core Development
> Christopher, I think this is only going to fly if you can show that
> there is some sort of significant performance improvement that can be
> made here without getting in the way of developers.

Look, I can tell you now that it's extremely unlikely any change of
this nature will make its way into core. The backward-compatibility
issues would just be too much. Plus those long classnames are
hideous. Finally, if we're going to make any changes to class
management, it would be better to leave it until we are committed to
requiring 5.3 and thereby making use of namespaces.

Marcus Nyeholt

unread,
Mar 2, 2011, 8:50:54 PM3/2/11
to silverst...@googlegroups.com
Yeah, fake namespacing using classnames is one of my pethates of zend framework and the like. 

Daniel Lindkvist

unread,
Mar 15, 2011, 7:32:53 AM3/15/11
to silverst...@googlegroups.com
I think I'd have problem getting approval to upgrade all our sites to ss 3.0 if it meant one month or more of writing and verifying non-revenue driving code per site :)

And yes, the class names are hideous. I've worked in a custom framework that used that standard before. It's almost impossible to discuss a class with peers, using its entire name, and the most interesting part of the name comes at the end.
It works out better for computers than humans :)

> --
> You received this message because you are subscribed to the Google Groups
> "SilverStripe Core Development" group.
> To post to this group, send email to silverst...@googlegroups.com.
> To unsubscribe from this group, send email to silverstripe-

> dev+uns...@googlegroups.com.

Reply all
Reply to author
Forward
0 new messages