Performance optimization

[tw2066]

Optimizing the map method can bring about decent performance

$mapper = new \JsonMapper();
$jsonStr = <<<'JSON'
{
    "name":"Sheldon Cooper",
    "address": {
        "street": "2311 N. Los Robles Avenue",
        "city": "Pasadena"
    }
}
JSON;

$jsonContact =  json_decode($jsonStr);


$start_time = microtime(true);
for ($i = 0; $i < 3000000; ++$i) {
    $contactObject = $mapper->map($jsonContact, new Contact());
}

It takes 6.86 seconds before modification
It takes 4.84 seconds after modification

However, the classMap function will no longer be supported

[cweiske]

Now that you don't seem to do any new commits to this patch:
Code will be faster if you remove features.
But the feature is there for a reason, and I won't remove the classmap feature because the code without it is faster.

[SvenRtbg]

This would need a proper name. The project consists of two classes, one of it an exception, as of now, and I believe it isn't ideal if you add random new class names into the root namespace.

[tw2066]

It's a good idea to use arrays or other names.

[tw2066]

Now that you don't seem to do any new commits to this patch: Code will be faster if you remove features. But the feature is there for a reason, and I won't remove the classmap feature because the code without it is faster.

The classMap method can be replaced with the set method.
Abandoning one small function to achieve a 30-40% performance improvement is a good solution.

[SvenRtbg]

I was trying to verify the performance claims, and noticed something odd:

Running your branch is indeed faster (caveat: you didn't provide the full code to simply copy&paste into a file, then run it, so I had to patch something that now represents MY performance test), however you claimed that you cannot include the classMap feature because that is the slow part.

I compared against the 5.0.0 master and did two different things:

1. immediately return null in getMappedType()
2. not calling the method at all

In both cases, the execution was slighly SLOWER compared to the unaltered master branch. Not by a far margin, fluctuation at around 1%, so easily explained by multitasking effects on my machine.

Anyways, I believe the classMap feature isn't to blame here, at least not for your suggested performance test case, as we'd be talking about one additional method call (getMappedType() is called in exactly two places, one per code path for either map() and mapArray()), and then executing the function with a call to isset() with an empty array, then some string comparisons, quickly returning the same type that was put in. So all in all not zero effort, but a minimum. And as illustrated, disabling or short cutting this function does not cut out a lot of execution time.

My measurements (long execution times likely caused by improper configuration of Xdebug):

• Your branch (with array commit): 38.807540 seconds
• Original code: 55.303229
• Quick return null: 55.715537
• Not call at all: 56.527193
• Back to original: 55.441865

As a general observation: The data structure you are enhancing is already representing analysis results in itself. So the return value of inspectProperty() is returning an array of details already, and you decided to not mess with that, but add another detail parameter containing an array of more details.

While I do admit I don't really like the list(...) approach to get back multiple values, I didn't have the energy to refactor until now, so it is the code we deal with. But some important thing now changes: I get from your claims that it will speed up things if many more detection details are stored in the cache. Still the code lacks a common approach how to structure these details. Using integer-indexed implicit arrays as return value container does not really cut it. While in theory it might be documentable using some PHPStan or other expression, it is not feasible in the long run.

Long story short: I am in favor of a proper cache info structure that hosts everything the JsonMapper knows about the target, including $hasProperty, $accessor, $type, $isNullable and your $typeAttribute.

And addtionally, keep the classMap feature. It might turn into an additional attribute, even though it is highly dynamic in nature as results may depend on both original type and value to be mapped, based on a closure.