NEW Update SS_ConfigStaticManifest to use Reflection

[micmania1]

This is the PR as discussed in #4029

It completely removes the static manifest and replaces the SS_ConfigStaticManifest implementation with Reflection.

Tests on PHP 5.5 showed no speed decrease but saved 0.5mb memory per page load on a base install, loading the home page.

[tractorcow]

Look at all that no code that's no longer there. :)

[micmania1]

Tests failed on PHP 5.3 - damn syntaxes.

[micmania1]

Note: If merged then some of this stuff will need to have a deprecation warning in future ss 3 releases.

[kinglozzer]

Why has this been added? Did the old manifest fix case sensitivity issues that the new version doesn’t?

[micmania1]

I'll get back to you XD (There is a reason but i'm not sure this is the right fix).

[micmania1]

I found a bug in submitting this pull request. DataObject::has_own_table() doesn't work with class names which don't exactly match the class name (ie. they're case-sensitive). This is because the "class name" in Config::inst()->get("ClassName', 'blah') is case-sensitive.

The following returns null for incorrectly cased class names:
http://api.silverstripe.org/3.1/source-class-DataObject.html#2483

The thing that highlighted this was the failure of DataObjectTest::testGetCaseInsensitive().

public function testGetCaseInsensitive() {
    // Test get_one() with bad case on the classname
    // Note: This will succeed only if the underlying DB server supports case-insensitive
    // table names (e.g. such as MySQL, but not SQLite3)
    if(!(DB::get_conn() instanceof MySQLDatabase)) {
        $this->markTestSkipped('MySQL only');
    }

    $subteam1 = DataObject::get_one('dataobjecttest_subteam', array(
        '"DataObjectTest_Team"."Title"' => 'Subteam 1'
    ), true);
    $this->assertNotEmpty($subteam1);
    $this->assertEquals($subteam1->Title, "Subteam 1");
}

This tries to prove that using a lowercase class name won't affect whether or not the result will be returned. The should have been failing, however reason this has passed up until this point is because the query is built incorrectly.

DataObject::has_own_table()'s failure has a knock-on affect which prevents ClassInfo::ancestry($class, $tableOnly = true) working correctly. When only looking up class ancestors with tables, the lower-case table name will be ignored (DataObject::has_own_table() == false) and no SQL will be added to fetch data from that table.

The given test has passed like so up until now:

class MyBaseClass extends DataObejct {
    private static $db = array('SomeField' => 'Varchar');
}

class MySubclass extends MyBaseClass {
    private static $db = array('SomeOtherField' => 'Varchar');
}

What happens when you query the subclass is that it gets completely ignored and only the base class is queried:

// Tries to detect whether or not 'mysubclass' has its own table. Doesn't find it so excludes from query.
// SELECT MyBaseClass.SomeField FROM MyBaseClass
DataObject::get_one('mysubclass');


// What SHOULD happen
// SELECT MyBaseClass.SomeField, MySubclass.SomeOtherField FROM MyBaseClass.SomeField LEFT JOIN MySubclass ON (MyBaseClass.ID = MySubclass.ID)
DataObject::get_one('mysubclass');

The fix i've implemented is in DataObject::has_own_table() and in ClassInfo::ancestry() so that only correct case-sensitive class names are used. This fixes the issue because the config system is case-sensitive when looking up class names, which is the root cause of this bug.

If you need me to split this out into 2 commits let me know.

[micmania1]

I see case-insensitivity has been suggested in #3381 - it would be good as long as performance doesn't take a hit.

[tractorcow]

@micmania1 you probably need to refactor that classname normalisation code you added into ClassInfo into a new method, so that other parts of the codebase are able to access it. Normalising the table name from a class is one such case.

[micmania1]

I'd like to wait and see a response in #3381 before this is merged given that this is easier to make case-insensitive.

@tractorcow I do agree, but I plan to tackle ClassInfo at some point so I'll give some proper thought to where that should belong then.

[nfauchelle]

@micmania1
Really like this commit. I've been using now on two websites (locally) for a couple of days now, and thought I would share some benchmarks.

PHP 5.5.18. No opcode caching, running in dev mode.
There are numerous things running on my laptop so not best conditions, but still....

tl;dr

• No noticeable speed change on the front end.
• Memory usage is improved, and is better the larger the site (smaller site 600k, larger site just over 1Mb)
• Flushing is faster, by about 2.3~ seconds. Better memory usage as well when flushing

Website A

• SS 3.1.13
• betterbuttons, gridfieldextensions
• mysite: 8 Pages, 2 DataObjects, 2 DataExtensions.

Accessing / after 25 runs.

Times:

	Min	Max	Avg	Median
Before	0.2429	0.2550	0.2474	0.245
After	0.2369	0.2653	0.2539	0.25435

Memory:

	Min	Max	Avg	Median
Before	25,690,112	25,690,112	25,690,112.00	25,690,112
After	24,903,680	24,903,680	24,903,680.00	24,903,680

Performing a flush on the command line 5 times.

Command line was used to avoid the token redirect.

Times:

	Min	Max	Avg	Median
Before	3.0450	3.0890	3.0648	3.05965
After	0.7980	0.8431	0.8171	0.8107

Memory:

	Min	Max	Avg	Median
Before	19,136,512	19,398,656	19,346,227.20	19,398,656
After	17,825,792	17,825,792	17,825,792.00	17,825,792

Website B:

SS 3.1.12
betterbuttons, gridfieldextensions, silverstripe-catalogmanager, silverstripe-lumberjack, silverstripe-menumanager
mysite: 9 Pages, 21 DataObjects, 4 Model Admin, 5 DataExtensions

Accessing / after 25 runs.

Times:

	Min	Max	Avg	Median
Before	0.4160	0.5906	0.4364	0.42385
After	0.4101	0.5017	0.4343	0.42735

Memory:

	Min	Max	Avg	Median
Before	27,525,120	27,787,264	27,588,034.56	27,525,120
After	26,476,544	26,476,544	26,476,544.00	26,476,544

Performing a flush on the command line 5 times.

Times:

	Min	Max	Avg	Median
Before	6.0492	6.2917	6.1396	6.09
After	3.6682	3.9522	3.7599	3.7007

Memory:

	Min	Max	Avg	Median
Before	28,049,408	28,049,408	28,049,408.00	28,049,408
After	26,738,688	27,000,832	26,948,403.20	27,000,832

So we can see a good speed up there with the flush! About 2.3~ seconds faster now. On the Website A there is a memory saving of 600k~ on general requests, on the larger site we are just over 1Mb.

There seems to be no real speed change in general use of the site.

Developing Website A with this patch has been great! It's so fast to flush and in many cases don't even need to.

[dhensby]

So, do we want to move this along? How about we rebase, please :)

[nfauchelle]

Yes please :) Would love to see it merged.

[micmania1]

Still waiting on #3381 @sminnee @hafriedlander

[tractorcow]

We've already addressed case-sensitivity issues in #3949. Do you still need to wait for #3381?

[micmania1]

Rebased!!!

[nfauchelle]

Woohoo!

[kinglozzer]