Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved performance of Uuid::fromString(), Uuid::fromBytes(), UuidInterface#toString(), UuidInterface#getBytes() #324

Merged

Conversation

Ocramius
Copy link
Contributor

This is mostly to store an initial state, before we go on and start manipulating the UUID library internals.

I'm going to try and iterate over the Uuid facade to improve its performance.

@Ocramius
Copy link
Contributor Author

Here are some preliminary results:

Before the patch (79589a2)

suite: 1343cb67556449c71377a85330bb2629ad983b6b, date: 2020-06-30, stime: 12:25:31
+---------------------------+-------------------------------------------+-----+------+-----+------------+-------------+-------------+-------------+-------------+----------+--------+-----------+
| benchmark                 | subject                                   | set | revs | its | mem_peak   | best        | mean        | mode        | worst       | stdev    | rstdev | diff      |
+---------------------------+-------------------------------------------+-----+------+-----+------------+-------------+-------------+-------------+-------------+----------+--------+-----------+
| UuidStringConversionBench | benchCreationOfTinyUuidFromString         | 0   | 5    | 5   | 6,568,592b | 55.200μs    | 57.840μs    | 59.202μs    | 60.000μs    | 1.973μs  | 3.41%  | 23.32x    |
| UuidStringConversionBench | benchCreationOfHugeUuidFromString         | 0   | 5    | 5   | 6,568,592b | 53.400μs    | 55.680μs    | 54.345μs    | 58.000μs    | 1.962μs  | 3.52%  | 22.45x    |
| UuidStringConversionBench | benchCreationOfUuidFromString             | 0   | 5    | 5   | 6,568,080b | 30.800μs    | 31.400μs    | 31.692μs    | 31.800μs    | 0.420μs  | 1.34%  | 12.66x    |
| UuidStringConversionBench | benchCreationOfPromiscuousUuidsFromString | 0   | 5    | 5   | 6,588,760b | 3,314.600μs | 3,447.880μs | 3,502.691μs | 3,568.200μs | 94.527μs | 2.74%  | 1,390.27x |
| UuidStringConversionBench | benchCreationOfTinyUuidFromBytes          | 0   | 5    | 5   | 6,568,544b | 48.600μs    | 50.480μs    | 49.082μs    | 53.000μs    | 1.908μs  | 3.78%  | 20.35x    |
| UuidStringConversionBench | benchCreationOfHugeUuidFromBytes          | 0   | 5    | 5   | 6,568,544b | 46.000μs    | 47.040μs    | 47.221μs    | 48.400μs    | 0.933μs  | 1.98%  | 18.97x    |
| UuidStringConversionBench | benchCreationOfUuidFromBytes              | 0   | 5    | 5   | 6,568,080b | 24.000μs    | 24.680μs    | 24.331μs    | 25.800μs    | 0.652μs  | 2.64%  | 9.95x     |
| UuidStringConversionBench | benchCreationOfPromiscuousUuidsFromBytes  | 0   | 5    | 5   | 6,583,960b | 2,321.600μs | 2,404.560μs | 2,366.477μs | 2,498.000μs | 67.761μs | 2.82%  | 969.58x   |
| UuidStringConversionBench | benchStringConversionOfTinyUuid           | 0   | 5    | 5   | 6,606,880b | 60.600μs    | 61.680μs    | 62.094μs    | 62.400μs    | 0.676μs  | 1.10%  | 24.87x    |
| UuidStringConversionBench | benchStringConversionOfHugeUuid           | 0   | 5    | 5   | 6,606,880b | 57.600μs    | 59.400μs    | 58.892μs    | 62.000μs    | 1.464μs  | 2.47%  | 23.95x    |
| UuidStringConversionBench | benchStringConversionOfUuid               | 0   | 5    | 5   | 6,606,880b | 49.800μs    | 52.320μs    | 52.740μs    | 54.600μs    | 1.700μs  | 3.25%  | 21.10x    |
| UuidStringConversionBench | benchStringConversionOfPromiscuousUuids   | 0   | 5    | 5   | 6,615,456b | 2,228.600μs | 2,315.320μs | 2,338.510μs | 2,376.400μs | 52.528μs | 2.27%  | 933.60x   |
| UuidStringConversionBench | benchBytesConversionOfTinyUuid            | 0   | 5    | 5   | 6,568,080b | 2.400μs     | 2.480μs     | 2.407μs     | 2.600μs     | 0.098μs  | 3.95%  | 1.00x     |
| UuidStringConversionBench | benchBytesConversionOfHugeUuid            | 0   | 5    | 5   | 6,568,080b | 2.400μs     | 2.520μs     | 2.593μs     | 2.600μs     | 0.098μs  | 3.89%  | 1.02x     |
| UuidStringConversionBench | benchBytesConversionOfUuid                | 0   | 5    | 5   | 6,568,080b | 2.400μs     | 2.520μs     | 2.593μs     | 2.600μs     | 0.098μs  | 3.89%  | 1.02x     |
| UuidStringConversionBench | benchBytesConversionOfPromiscuousUuids    | 0   | 5    | 5   | 6,568,088b | 237.600μs   | 248.880μs   | 249.655μs   | 259.600μs   | 7.929μs  | 3.19%  | 100.35x   |
+---------------------------+-------------------------------------------+-----+------+-----+------------+-------------+-------------+-------------+-------------+----------+--------+-----------+

First optimization of Uuid#fromString() (7ef9270)

suite: 1343cb6780ff9b628371dac8458a2220cc9e933a, date: 2020-06-30, stime: 14:40:01
+---------------------------+-------------------------------------------+-----+------+-----+------------+-------------+-------------+-------------+-------------+----------+--------+-----------+
| benchmark                 | subject                                   | set | revs | its | mem_peak   | best        | mean        | mode        | worst       | stdev    | rstdev | diff      |
+---------------------------+-------------------------------------------+-----+------+-----+------------+-------------+-------------+-------------+-------------+----------+--------+-----------+
| UuidStringConversionBench | benchCreationOfTinyUuidFromString         | 0   | 5    | 5   | 6,093,296b | 2.800μs     | 2.880μs     | 2.807μs     | 3.000μs     | 0.098μs  | 3.40%  | 2.40x     |
| UuidStringConversionBench | benchCreationOfHugeUuidFromString         | 0   | 5    | 5   | 6,093,296b | 3.000μs     | 3.000μs     | 3.000μs     | 3.000μs     | 0.000μs  | 0.00%  | 2.50x     |
| UuidStringConversionBench | benchCreationOfUuidFromString             | 0   | 5    | 5   | 6,093,288b | 3.000μs     | 3.000μs     | 3.000μs     | 3.000μs     | 0.000μs  | 0.00%  | 2.50x     |
| UuidStringConversionBench | benchCreationOfPromiscuousUuidsFromString | 0   | 5    | 5   | 6,093,312b | 165.400μs   | 173.920μs   | 177.489μs   | 181.000μs   | 6.377μs  | 3.67%  | 144.93x   |
| UuidStringConversionBench | benchCreationOfTinyUuidFromBytes          | 0   | 5    | 5   | 6,635,032b | 864.200μs   | 900.320μs   | 899.959μs   | 937.000μs   | 23.092μs | 2.56%  | 750.27x   |
| UuidStringConversionBench | benchCreationOfHugeUuidFromBytes          | 0   | 5    | 5   | 6,635,032b | 846.600μs   | 873.320μs   | 851.364μs   | 912.400μs   | 29.786μs | 3.41%  | 727.77x   |
| UuidStringConversionBench | benchCreationOfUuidFromBytes              | 0   | 5    | 5   | 6,602,080b | 788.600μs   | 825.520μs   | 834.377μs   | 843.000μs   | 18.992μs | 2.30%  | 687.93x   |
| UuidStringConversionBench | benchCreationOfPromiscuousUuidsFromBytes  | 0   | 5    | 5   | 6,610,352b | 3,611.600μs | 3,722.960μs | 3,779.753μs | 3,830.800μs | 87.216μs | 2.34%  | 3,102.47x |
| UuidStringConversionBench | benchStringConversionOfTinyUuid           | 0   | 5    | 5   | 6,093,288b | 1.200μs     | 1.200μs     | 1.200μs     | 1.200μs     | 0.000μs  | 0.00%  | 1.00x     |
| UuidStringConversionBench | benchStringConversionOfHugeUuid           | 0   | 5    | 5   | 6,093,288b | 1.200μs     | 1.200μs     | 1.200μs     | 1.200μs     | 0.000μs  | 0.00%  | 1.00x     |
| UuidStringConversionBench | benchStringConversionOfUuid               | 0   | 5    | 5   | 6,093,288b | 1.400μs     | 1.400μs     | 1.400μs     | 1.400μs     | 0.000μs  | 0.00%  | 1.17x     |
| UuidStringConversionBench | benchStringConversionOfPromiscuousUuids   | 0   | 5    | 5   | 6,093,296b | 118.200μs   | 118.920μs   | 118.391μs   | 119.800μs   | 0.733μs  | 0.62%  | 99.10x    |
| UuidStringConversionBench | benchBytesConversionOfTinyUuid            | 0   | 5    | 5   | 6,093,288b | 2.000μs     | 2.000μs     | 2.000μs     | 2.000μs     | 0.000μs  | 0.00%  | 1.67x     |
| UuidStringConversionBench | benchBytesConversionOfHugeUuid            | 0   | 5    | 5   | 6,093,288b | 1.800μs     | 1.800μs     | 1.800μs     | 1.800μs     | 0.000μs  | 0.00%  | 1.50x     |
| UuidStringConversionBench | benchBytesConversionOfUuid                | 0   | 5    | 5   | 6,093,288b | 1.800μs     | 1.800μs     | 1.800μs     | 1.800μs     | 0.000μs  | 0.00%  | 1.50x     |
| UuidStringConversionBench | benchBytesConversionOfPromiscuousUuids    | 0   | 5    | 5   | 6,093,296b | 172.400μs   | 179.280μs   | 181.936μs   | 183.400μs   | 4.212μs  | 2.35%  | 149.40x   |
+---------------------------+-------------------------------------------+-----+------+-----+------------+-------------+-------------+-------------+-------------+----------+--------+-----------+

After optimizing Uuid#fromBytes() (9067fe6):

suite: 1343cb60feaa58e32feac2341b646153dbf33c84, date: 2020-06-30, stime: 15:01:49
+---------------------------+-------------------------------------------+-----+------+-----+------------+-----------+-----------+-----------+-----------+---------+--------+---------+
| benchmark                 | subject                                   | set | revs | its | mem_peak   | best      | mean      | mode      | worst     | stdev   | rstdev | diff    |
+---------------------------+-------------------------------------------+-----+------+-----+------------+-----------+-----------+-----------+-----------+---------+--------+---------+
| UuidStringConversionBench | benchCreationOfTinyUuidFromString         | 0   | 5    | 5   | 6,096,280b | 2.800μs   | 2.880μs   | 2.807μs   | 3.000μs   | 0.098μs | 3.40%  | 2.40x   |
| UuidStringConversionBench | benchCreationOfHugeUuidFromString         | 0   | 5    | 5   | 6,096,280b | 2.800μs   | 2.800μs   | 2.800μs   | 2.800μs   | 0.000μs | 0.00%  | 2.33x   |
| UuidStringConversionBench | benchCreationOfUuidFromString             | 0   | 5    | 5   | 6,096,272b | 2.600μs   | 2.600μs   | 2.600μs   | 2.600μs   | 0.000μs | 0.00%  | 2.17x   |
| UuidStringConversionBench | benchCreationOfPromiscuousUuidsFromString | 0   | 5    | 5   | 6,096,296b | 157.200μs | 164.960μs | 165.046μs | 172.800μs | 5.100μs | 3.09%  | 137.47x |
| UuidStringConversionBench | benchCreationOfTinyUuidFromBytes          | 0   | 5    | 5   | 6,096,280b | 5.400μs   | 5.520μs   | 5.593μs   | 5.600μs   | 0.098μs | 1.77%  | 4.60x   |
| UuidStringConversionBench | benchCreationOfHugeUuidFromBytes          | 0   | 5    | 5   | 6,096,280b | 5.200μs   | 5.400μs   | 5.515μs   | 5.600μs   | 0.179μs | 3.31%  | 4.50x   |
| UuidStringConversionBench | benchCreationOfUuidFromBytes              | 0   | 5    | 5   | 6,096,272b | 5.400μs   | 5.440μs   | 5.400μs   | 5.600μs   | 0.080μs | 1.47%  | 4.53x   |
| UuidStringConversionBench | benchCreationOfPromiscuousUuidsFromBytes  | 0   | 5    | 5   | 6,096,296b | 399.800μs | 406.520μs | 409.971μs | 412.000μs | 4.933μs | 1.21%  | 338.77x |
| UuidStringConversionBench | benchStringConversionOfTinyUuid           | 0   | 5    | 5   | 6,096,272b | 1.200μs   | 1.200μs   | 1.200μs   | 1.200μs   | 0.000μs | 0.00%  | 1.00x   |
| UuidStringConversionBench | benchStringConversionOfHugeUuid           | 0   | 5    | 5   | 6,096,272b | 1.200μs   | 1.200μs   | 1.200μs   | 1.200μs   | 0.000μs | 0.00%  | 1.00x   |
| UuidStringConversionBench | benchStringConversionOfUuid               | 0   | 5    | 5   | 6,096,272b | 1.200μs   | 1.200μs   | 1.200μs   | 1.200μs   | 0.000μs | 0.00%  | 1.00x   |
| UuidStringConversionBench | benchStringConversionOfPromiscuousUuids   | 0   | 5    | 5   | 6,096,280b | 111.000μs | 116.640μs | 118.045μs | 120.400μs | 3.407μs | 2.92%  | 97.20x  |
| UuidStringConversionBench | benchBytesConversionOfTinyUuid            | 0   | 5    | 5   | 6,096,272b | 1.800μs   | 1.800μs   | 1.800μs   | 1.800μs   | 0.000μs | 0.00%  | 1.50x   |
| UuidStringConversionBench | benchBytesConversionOfHugeUuid            | 0   | 5    | 5   | 6,096,272b | 1.800μs   | 1.800μs   | 1.800μs   | 1.800μs   | 0.000μs | 0.00%  | 1.50x   |
| UuidStringConversionBench | benchBytesConversionOfUuid                | 0   | 5    | 5   | 6,096,272b | 1.800μs   | 1.800μs   | 1.800μs   | 1.800μs   | 0.000μs | 0.00%  | 1.50x   |
| UuidStringConversionBench | benchBytesConversionOfPromiscuousUuids    | 0   | 5    | 5   | 6,096,280b | 173.000μs | 177.760μs | 178.601μs | 182.000μs | 3.045μs | 1.71%  | 148.13x |
+---------------------------+-------------------------------------------+-----+------+-----+------------+-----------+-----------+-----------+-----------+---------+--------+---------+

Overall, a speedup of ~20 times is observed for these traditional use-cases.

@Ocramius Ocramius changed the title WIP: Minimal benchmarks around Uuid::fromString(), Uuid::fromBytes(), Uuid#toString(), Uuid#getBytes() Minimal benchmarks around Uuid::fromString(), Uuid::fromBytes(), Uuid#toString(), Uuid#getBytes() Jun 30, 2020
@@ -1431,7 +1428,6 @@ public function providePythonTests(): array
'clock_seq' => '27b8',
'variant' => Uuid::RFC_4122,
'version' => Uuid::UUID_TYPE_HASH_MD5,
'class' => UuidV3::class,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I don't think that changing the type of Uuid::fromString() is a BC break here, since the interfaced type is still respected

@Ocramius Ocramius changed the title Minimal benchmarks around Uuid::fromString(), Uuid::fromBytes(), Uuid#toString(), Uuid#getBytes() Improved performance of Uuid::fromString(), Uuid::fromBytes(), UuidInterface#toString(), UuidInterface#getBytes() Jun 30, 2020
@Ocramius
Copy link
Contributor Author

Obviously, all of this is easily subject to refactoring.

In my opinion, two big improvements for 5.0.0 could be:

  1. make a *Factory specific for FieldsInterface
  2. remove Uuid#setFactory() (deprecate it in 4.1.0 or 4.2.0?)
  3. overall simplify UuidInterface to the bone: if someone needs customization, they should use different entry-points than the default ones.

@GGrobot
Copy link

GGrobot commented Jul 1, 2020

Wtf you really did this shit, good job! 👏

src/Uuid.php Outdated Show resolved Hide resolved
@Ocramius
Copy link
Contributor Author

Ocramius commented Jul 3, 2020

Just noticed one problem with this patch that I could only detect in a larger test suite. The following will fail:

$id = Uuid::uuid4();

self::assertEquals($id, Uuid::fromString($id->toString()));

This is because we no longer create a UuidV4 instance, but rather a lazy instance.

I think I will change the various Uuid::uuid*() constructors to return this data structure as well, so the problem is mitigated: this will basically make all the specialty UuidInterface implementations almost unused in all scenarios, but I promise it's for a good cause ^_^''

@Ocramius
Copy link
Contributor Author

Ocramius commented Jul 3, 2020

I've added tests that ensure that Uuid::uuid4() (and other generator methods) produce the same type as Uuid::fromString() and Uuid::fromBytes().

In applying this change, we managed to get another 10% perf out of that part of the logic 👍

suite: 1343cffa3378502ba9d02dae85352a32436adc72, date: 2020-07-03, stime: 14:25:52
+---------------------+-------------------------------------------------------------------------+-----+------+-----+------------+-------------+-------------+-------------+-------------+----------+--------+-------+
| benchmark           | subject                                                                 | set | revs | its | mem_peak   | best        | mean        | mode        | worst       | stdev    | rstdev | diff  |
+---------------------+-------------------------------------------------------------------------+-----+------+-----+------------+-------------+-------------+-------------+-------------+----------+--------+-------+
| UuidGenerationBench | benchUuid1GenerationWithoutParameters                                   | 0   | 5    | 5   | 7,321,336b | 1,257.800μs | 1,305.960μs | 1,300.117μs | 1,363.800μs | 37.182μs | 2.85%  | 1.80x |
| UuidGenerationBench | benchUuid1GenerationWithNode                                            | 0   | 5    | 5   | 7,321,112b | 1,180.600μs | 1,198.840μs | 1,190.562μs | 1,231.000μs | 17.747μs | 1.48%  | 1.65x |
| UuidGenerationBench | benchUuid1GenerationWithNodeAndClockSequence                            | 0   | 5    | 5   | 7,321,136b | 1,169.800μs | 1,199.920μs | 1,195.882μs | 1,237.800μs | 25.602μs | 2.13%  | 1.65x |
| UuidGenerationBench | benchUuid2GenerationWithDomainAndLocalIdentifier                        | 0   | 5    | 5   | 7,616,640b | 1,490.600μs | 1,549.800μs | 1,540.590μs | 1,621.600μs | 42.612μs | 2.75%  | 2.13x |
| UuidGenerationBench | benchUuid2GenerationWithDomainAndLocalIdentifierAndNode                 | 0   | 5    | 5   | 7,616,424b | 1,462.600μs | 1,502.080μs | 1,482.101μs | 1,566.400μs | 37.350μs | 2.49%  | 2.07x |
| UuidGenerationBench | benchUuid2GenerationWithDomainAndLocalIdentifierAndNodeAndClockSequence | 0   | 5    | 5   | 7,616,440b | 1,451.200μs | 1,496.800μs | 1,500.449μs | 1,536.800μs | 28.019μs | 1.87%  | 2.06x |
| UuidGenerationBench | benchUuid3Generation                                                    | 0   | 5    | 5   | 7,306,120b | 707.400μs   | 732.120μs   | 736.447μs   | 750.800μs   | 14.404μs | 1.97%  | 1.01x |
| UuidGenerationBench | benchUuid4Generation                                                    | 0   | 5    | 5   | 7,306,040b | 707.400μs   | 726.120μs   | 730.382μs   | 739.400μs   | 11.551μs | 1.59%  | 1.00x |
| UuidGenerationBench | benchUuid5Generation                                                    | 0   | 5    | 5   | 7,306,136b | 724.400μs   | 753.480μs   | 763.525μs   | 778.000μs   | 19.706μs | 2.62%  | 1.04x |
| UuidGenerationBench | benchUuid6GenerationWithoutParameters                                   | 0   | 5    | 5   | 7,333,816b | 1,170.000μs | 1,180.600μs | 1,175.539μs | 1,201.800μs | 11.071μs | 0.94%  | 1.63x |
| UuidGenerationBench | benchUuid6GenerationWithNode                                            | 0   | 5    | 5   | 7,333,592b | 1,223.200μs | 1,243.640μs | 1,228.978μs | 1,303.000μs | 29.885μs | 2.40%  | 1.71x |
| UuidGenerationBench | benchUuid6GenerationWithNodeAndClockSequence                            | 0   | 5    | 5   | 7,333,616b | 1,168.800μs | 1,229.720μs | 1,233.105μs | 1,287.000μs | 38.927μs | 3.17%  | 1.69x |
+---------------------+-------------------------------------------------------------------------+-----+------+-----+------------+-------------+-------------+-------------+-------------+----------+--------+-------+
suite: 1343cffe143d3e7c4ac99f978a2cfb2be04e9e05, date: 2020-07-03, stime: 14:26:09
+---------------------+-------------------------------------------------------------------------+-----+------+-----+------------+-------------+-------------+-------------+-------------+----------+--------+-------+
| benchmark           | subject                                                                 | set | revs | its | mem_peak   | best        | mean        | mode        | worst       | stdev    | rstdev | diff  |
+---------------------+-------------------------------------------------------------------------+-----+------+-----+------------+-------------+-------------+-------------+-------------+----------+--------+-------+
| UuidGenerationBench | benchUuid1GenerationWithoutParameters                                   | 0   | 5    | 5   | 7,274,696b | 1,211.200μs | 1,247.040μs | 1,232.343μs | 1,285.200μs | 27.483μs | 2.20%  | 1.96x |
| UuidGenerationBench | benchUuid1GenerationWithNode                                            | 0   | 5    | 5   | 7,274,472b | 1,133.000μs | 1,143.720μs | 1,138.861μs | 1,153.800μs | 8.181μs  | 0.72%  | 1.80x |
| UuidGenerationBench | benchUuid1GenerationWithNodeAndClockSequence                            | 0   | 5    | 5   | 7,274,496b | 1,060.200μs | 1,099.680μs | 1,120.153μs | 1,134.200μs | 30.024μs | 2.73%  | 1.73x |
| UuidGenerationBench | benchUuid2GenerationWithDomainAndLocalIdentifier                        | 0   | 5    | 5   | 7,584,888b | 1,392.200μs | 1,431.400μs | 1,422.987μs | 1,484.200μs | 29.745μs | 2.08%  | 2.25x |
| UuidGenerationBench | benchUuid2GenerationWithDomainAndLocalIdentifierAndNode                 | 0   | 5    | 5   | 7,584,888b | 1,328.200μs | 1,337.440μs | 1,340.858μs | 1,343.600μs | 5.856μs  | 0.44%  | 2.10x |
| UuidGenerationBench | benchUuid2GenerationWithDomainAndLocalIdentifierAndNodeAndClockSequence | 0   | 5    | 5   | 7,584,904b | 1,279.400μs | 1,318.560μs | 1,338.580μs | 1,352.800μs | 30.642μs | 2.32%  | 2.07x |
| UuidGenerationBench | benchUuid3Generation                                                    | 0   | 5    | 5   | 7,263,144b | 621.800μs   | 643.680μs   | 636.209μs   | 669.000μs   | 16.620μs | 2.58%  | 1.01x |
| UuidGenerationBench | benchUuid4Generation                                                    | 0   | 5    | 5   | 7,263,144b | 622.800μs   | 650.880μs   | 657.625μs   | 667.400μs   | 15.414μs | 2.37%  | 1.02x |
| UuidGenerationBench | benchUuid5Generation                                                    | 0   | 5    | 5   | 7,263,192b | 611.000μs   | 635.520μs   | 643.764μs   | 649.400μs   | 14.487μs | 2.28%  | 1.00x |
| UuidGenerationBench | benchUuid6GenerationWithoutParameters                                   | 0   | 5    | 5   | 7,274,696b | 1,111.600μs | 1,139.720μs | 1,143.405μs | 1,161.000μs | 17.865μs | 1.57%  | 1.79x |
| UuidGenerationBench | benchUuid6GenerationWithNode                                            | 0   | 5    | 5   | 7,274,472b | 1,109.400μs | 1,162.840μs | 1,173.095μs | 1,212.400μs | 35.642μs | 3.07%  | 1.83x |
| UuidGenerationBench | benchUuid6GenerationWithNodeAndClockSequence                            | 0   | 5    | 5   | 7,274,496b | 1,002.400μs | 1,044.520μs | 1,048.629μs | 1,079.600μs | 26.235μs | 2.51%  | 1.64x |
+---------------------+-------------------------------------------------------------------------+-----+------+-----+------------+-------------+-------------+-------------+-------------+----------+--------+-------+

Note: I'm not fond of these hacks, but I think the root cause is still Uuid::setFactory(): it has to go as soon as you can make it go away.

Copy link
Owner

@ramsey ramsey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything here looks great. I see the problem with the factory now. I believe the reason for being able to swap the factory was primarily for testing purposes, but I see now how this design is flawed, so it makes sense to remove that in 5.0.0.

I only have one question about the unwrap() method.

private function unwrap(): UuidInterface
{
return (new UuidFactory())
->fromString($this->uuid);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this make more sense to store to a property so that unwrap() only calls (new UuidFactory())->fromString() once per instance?

e.g.,

if ($this->uuidInstance === null) {
    $this->uuidInstance = (new UuidFactory())->fromString($this->uuid);
}

return $this->uuidInstance;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can most certainly be done: possibly worth writing a benchmark for those paths.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect it’d be beneficial when dealing with one Uuid instance and operating on it using multiple methods, but it might be detrimental when using multiple Uuid instances and calling only a single method on each (such as in a loop).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly memory impact, but only when looking at details such as UUID internals

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking further about this, the only affected API is getFields, as everything else is deprecated.

If we add a new private field, we do slow down serialization/deserialization, but that's something I's have to benchmark as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added benchmarks and applied this optimization: it seems like an acceptable trade-off.

Here are the results before/after:

+---------------------------+-------------------------------------------------------------------------+-----+------+-----+------------+-------------+-------------+-------------+-------------+----------+--------+-----------+
| benchmark                 | subject                                                                 | set | revs | its | mem_peak   | best        | mean        | mode        | worst       | stdev    | rstdev | diff      |
+---------------------------+-------------------------------------------------------------------------+-----+------+-----+------------+-------------+-------------+-------------+-------------+----------+--------+-----------+
| UuidStringConversionBench | benchCreationOfTinyUuidFromString                                       | 0   | 5    | 5   | 6,109,896b | 2.800μs     | 2.800μs     | 2.800μs     | 2.800μs     | 0.000μs  | 0.00%  | 2.33x     |
| UuidStringConversionBench | benchCreationOfHugeUuidFromString                                       | 0   | 5    | 5   | 6,109,896b | 2.800μs     | 2.800μs     | 2.800μs     | 2.800μs     | 0.000μs  | 0.00%  | 2.33x     |
| UuidStringConversionBench | benchCreationOfUuidFromString                                           | 0   | 5    | 5   | 6,109,888b | 2.800μs     | 2.880μs     | 2.807μs     | 3.000μs     | 0.098μs  | 3.40%  | 2.40x     |
| UuidStringConversionBench | benchCreationOfPromiscuousUuidsFromString                               | 0   | 5    | 5   | 6,109,912b | 155.800μs   | 161.200μs   | 158.667μs   | 169.000μs   | 4.735μs  | 2.94%  | 134.33x   |
| UuidStringConversionBench | benchCreationOfTinyUuidFromBytes                                        | 0   | 5    | 5   | 6,109,896b | 5.800μs     | 5.840μs     | 5.800μs     | 6.000μs     | 0.080μs  | 1.37%  | 4.87x     |
| UuidStringConversionBench | benchCreationOfHugeUuidFromBytes                                        | 0   | 5    | 5   | 6,109,896b | 5.400μs     | 5.480μs     | 5.407μs     | 5.600μs     | 0.098μs  | 1.79%  | 4.57x     |
| UuidStringConversionBench | benchCreationOfUuidFromBytes                                            | 0   | 5    | 5   | 6,109,888b | 5.200μs     | 5.400μs     | 5.515μs     | 5.600μs     | 0.179μs  | 3.31%  | 4.50x     |
| UuidStringConversionBench | benchCreationOfPromiscuousUuidsFromBytes                                | 0   | 5    | 5   | 6,109,912b | 380.800μs   | 393.760μs   | 392.218μs   | 409.400μs   | 9.927μs  | 2.52%  | 328.13x   |
| UuidStringConversionBench | benchStringConversionOfTinyUuid                                         | 0   | 5    | 5   | 6,109,888b | 1.200μs     | 1.200μs     | 1.200μs     | 1.200μs     | 0.000μs  | 0.00%  | 1.00x     |
| UuidStringConversionBench | benchStringConversionOfHugeUuid                                         | 0   | 5    | 5   | 6,109,888b | 1.200μs     | 1.200μs     | 1.200μs     | 1.200μs     | 0.000μs  | 0.00%  | 1.00x     |
| UuidStringConversionBench | benchStringConversionOfUuid                                             | 0   | 5    | 5   | 6,109,888b | 1.200μs     | 1.200μs     | 1.200μs     | 1.200μs     | 0.000μs  | 0.00%  | 1.00x     |
| UuidStringConversionBench | benchStringConversionOfPromiscuousUuids                                 | 0   | 5    | 5   | 6,109,896b | 115.200μs   | 116.680μs   | 115.792μs   | 120.000μs   | 1.774μs  | 1.52%  | 97.23x    |
| UuidStringConversionBench | benchBytesConversionOfTinyUuid                                          | 0   | 5    | 5   | 6,109,888b | 2.000μs     | 2.000μs     | 2.000μs     | 2.000μs     | 0.000μs  | 0.00%  | 1.67x     |
| UuidStringConversionBench | benchBytesConversionOfHugeUuid                                          | 0   | 5    | 5   | 6,109,888b | 2.000μs     | 2.000μs     | 2.000μs     | 2.000μs     | 0.000μs  | 0.00%  | 1.67x     |
| UuidStringConversionBench | benchBytesConversionOfUuid                                              | 0   | 5    | 5   | 6,109,888b | 2.000μs     | 2.000μs     | 2.000μs     | 2.000μs     | 0.000μs  | 0.00%  | 1.67x     |
| UuidStringConversionBench | benchBytesConversionOfPromiscuousUuids                                  | 0   | 5    | 5   | 6,109,896b | 182.000μs   | 189.200μs   | 191.148μs   | 194.400μs   | 4.358μs  | 2.30%  | 157.67x   |
| UuidFieldExtractionBench  | benchGetFields                                                          | 0   | 5    | 5   | 6,567,712b | 883.200μs   | 922.640μs   | 922.594μs   | 963.400μs   | 27.870μs | 3.02%  | 768.87x   |
| UuidFieldExtractionBench  | benchGetFields10Times                                                   | 0   | 5    | 5   | 6,567,720b | 1,501.600μs | 1,546.880μs | 1,537.530μs | 1,603.600μs | 33.694μs | 2.18%  | 1,289.07x |
| UuidFieldExtractionBench  | benchGetHex                                                             | 0   | 5    | 5   | 6,578,592b | 910.800μs   | 956.920μs   | 967.285μs   | 985.000μs   | 25.444μs | 2.66%  | 797.43x   |
| UuidFieldExtractionBench  | benchGetHex10Times                                                      | 0   | 5    | 5   | 6,578,600b | 1,633.400μs | 1,666.440μs | 1,642.168μs | 1,737.600μs | 41.014μs | 2.46%  | 1,388.70x |
| UuidFieldExtractionBench  | benchGetInteger                                                         | 0   | 5    | 5   | 6,912,320b | 1,101.600μs | 1,129.680μs | 1,146.378μs | 1,152.000μs | 22.803μs | 2.02%  | 941.40x   |
| UuidFieldExtractionBench  | benchGetInteger10Times                                                  | 0   | 5    | 5   | 6,912,328b | 2,130.800μs | 2,176.040μs | 2,165.507μs | 2,237.000μs | 35.627μs | 1.64%  | 1,813.37x |
| UuidGenerationBench       | benchUuid1GenerationWithoutParameters                                   | 0   | 5    | 5   | 6,556,760b | 1,144.400μs | 1,188.760μs | 1,184.077μs | 1,223.600μs | 30.154μs | 2.54%  | 990.63x   |
| UuidGenerationBench       | benchUuid1GenerationWithNode                                            | 0   | 5    | 5   | 6,556,536b | 1,181.400μs | 1,202.080μs | 1,204.126μs | 1,219.600μs | 12.196μs | 1.01%  | 1,001.73x |
| UuidGenerationBench       | benchUuid1GenerationWithNodeAndClockSequence                            | 0   | 5    | 5   | 6,556,560b | 1,043.200μs | 1,065.720μs | 1,048.704μs | 1,100.600μs | 24.119μs | 2.26%  | 888.10x   |
| UuidGenerationBench       | benchUuid2GenerationWithDomainAndLocalIdentifier                        | 0   | 5    | 5   | 6,866,968b | 1,387.400μs | 1,403.000μs | 1,394.944μs | 1,436.200μs | 17.327μs | 1.23%  | 1,169.17x |
| UuidGenerationBench       | benchUuid2GenerationWithDomainAndLocalIdentifierAndNode                 | 0   | 5    | 5   | 6,866,968b | 1,306.200μs | 1,360.120μs | 1,380.088μs | 1,393.600μs | 33.230μs | 2.44%  | 1,133.43x |
| UuidGenerationBench       | benchUuid2GenerationWithDomainAndLocalIdentifierAndNodeAndClockSequence | 0   | 5    | 5   | 6,866,984b | 1,320.600μs | 1,372.280μs | 1,387.935μs | 1,412.600μs | 35.368μs | 2.58%  | 1,143.57x |
| UuidGenerationBench       | benchUuid3Generation                                                    | 0   | 5    | 5   | 6,545,208b | 672.200μs   | 694.560μs   | 689.754μs   | 718.200μs   | 16.013μs | 2.31%  | 578.80x   |
| UuidGenerationBench       | benchUuid4Generation                                                    | 0   | 5    | 5   | 6,545,208b | 659.400μs   | 667.360μs   | 664.726μs   | 676.200μs   | 6.056μs  | 0.91%  | 556.13x   |
| UuidGenerationBench       | benchUuid5Generation                                                    | 0   | 5    | 5   | 6,545,256b | 640.200μs   | 652.640μs   | 654.649μs   | 665.400μs   | 9.250μs  | 1.42%  | 543.87x   |
| UuidGenerationBench       | benchUuid6GenerationWithoutParameters                                   | 0   | 5    | 5   | 6,556,760b | 1,191.400μs | 1,238.760μs | 1,259.899μs | 1,267.000μs | 30.777μs | 2.48%  | 1,032.30x |
| UuidGenerationBench       | benchUuid6GenerationWithNode                                            | 0   | 5    | 5   | 6,556,536b | 1,088.800μs | 1,112.600μs | 1,097.812μs | 1,168.200μs | 29.407μs | 2.64%  | 927.17x   |
| UuidGenerationBench       | benchUuid6GenerationWithNodeAndClockSequence                            | 0   | 5    | 5   | 6,556,560b | 1,099.000μs | 1,112.520μs | 1,104.953μs | 1,138.000μs | 14.597μs | 1.31%  | 927.10x   |
| UuidSerializationBench    | benchSerializationOfTinyUuid                                            | 0   | 5    | 5   | 6,103,264b | 1.600μs     | 1.600μs     | 1.600μs     | 1.600μs     | 0.000μs  | 0.00%  | 1.33x     |
| UuidSerializationBench    | benchSerializationOfHugeUuid                                            | 0   | 5    | 5   | 6,103,264b | 1.600μs     | 1.600μs     | 1.600μs     | 1.600μs     | 0.000μs  | 0.00%  | 1.33x     |
| UuidSerializationBench    | benchSerializationOfUuid                                                | 0   | 5    | 5   | 6,103,264b | 1.600μs     | 1.600μs     | 1.600μs     | 1.600μs     | 0.000μs  | 0.00%  | 1.33x     |
| UuidSerializationBench    | benchSerializationOfPromiscuousUuids                                    | 0   | 5    | 5   | 6,121,448b | 64.000μs    | 64.480μs    | 64.176μs    | 65.000μs    | 0.449μs  | 0.70%  | 53.73x    |
| UuidSerializationBench    | benchDeSerializationOfTinyUuid                                          | 0   | 5    | 5   | 6,103,264b | 2.200μs     | 2.200μs     | 2.200μs     | 2.200μs     | 0.000μs  | 0.00%  | 1.83x     |
| UuidSerializationBench    | benchDeSerializationOfHugeUuid                                          | 0   | 5    | 5   | 6,103,264b | 2.200μs     | 2.200μs     | 2.200μs     | 2.200μs     | 0.000μs  | 0.00%  | 1.83x     |
| UuidSerializationBench    | benchDeSerializationOfUuid                                              | 0   | 5    | 5   | 6,103,264b | 2.200μs     | 2.200μs     | 2.200μs     | 2.200μs     | 0.000μs  | 0.00%  | 1.83x     |
| UuidSerializationBench    | benchDeSerializationOfPromiscuousUuids                                  | 0   | 5    | 5   | 6,119,456b | 85.200μs    | 86.360μs    | 86.483μs    | 87.400μs    | 0.742μs  | 0.86%  | 71.97x    |
+---------------------------+-------------------------------------------------------------------------+-----+------+-----+------------+-------------+-------------+-------------+-------------+----------+--------+-----------+

+---------------------------+-------------------------------------------------------------------------+-----+------+-----+------------+-------------+-------------+-------------+-------------+----------+--------+-----------+
| benchmark                 | subject                                                                 | set | revs | its | mem_peak   | best        | mean        | mode        | worst       | stdev    | rstdev | diff      |
+---------------------------+-------------------------------------------------------------------------+-----+------+-----+------------+-------------+-------------+-------------+-------------+----------+--------+-----------+
| UuidStringConversionBench | benchCreationOfTinyUuidFromString                                       | 0   | 5    | 5   | 6,153,936b | 2.600μs     | 2.720μs     | 2.793μs     | 2.800μs     | 0.098μs  | 3.60%  | 2.72x     |
| UuidStringConversionBench | benchCreationOfHugeUuidFromString                                       | 0   | 5    | 5   | 6,153,936b | 2.400μs     | 2.480μs     | 2.407μs     | 2.600μs     | 0.098μs  | 3.95%  | 2.48x     |
| UuidStringConversionBench | benchCreationOfUuidFromString                                           | 0   | 5    | 5   | 6,153,928b | 2.600μs     | 2.680μs     | 2.607μs     | 2.800μs     | 0.098μs  | 3.66%  | 2.68x     |
| UuidStringConversionBench | benchCreationOfPromiscuousUuidsFromString                               | 0   | 5    | 5   | 6,153,952b | 159.400μs   | 165.120μs   | 167.367μs   | 171.200μs   | 4.446μs  | 2.69%  | 165.12x   |
| UuidStringConversionBench | benchCreationOfTinyUuidFromBytes                                        | 0   | 5    | 5   | 6,153,936b | 5.200μs     | 5.280μs     | 5.207μs     | 5.400μs     | 0.098μs  | 1.86%  | 5.28x     |
| UuidStringConversionBench | benchCreationOfHugeUuidFromBytes                                        | 0   | 5    | 5   | 6,153,936b | 5.200μs     | 5.360μs     | 5.322μs     | 5.600μs     | 0.150μs  | 2.79%  | 5.36x     |
| UuidStringConversionBench | benchCreationOfUuidFromBytes                                            | 0   | 5    | 5   | 6,153,928b | 5.200μs     | 5.240μs     | 5.200μs     | 5.400μs     | 0.080μs  | 1.53%  | 5.24x     |
| UuidStringConversionBench | benchCreationOfPromiscuousUuidsFromBytes                                | 0   | 5    | 5   | 6,153,952b | 365.400μs   | 381.240μs   | 385.598μs   | 393.600μs   | 11.033μs | 2.89%  | 381.24x   |
| UuidStringConversionBench | benchStringConversionOfTinyUuid                                         | 0   | 5    | 5   | 6,153,928b | 1.000μs     | 1.000μs     | 1.000μs     | 1.000μs     | 0.000μs  | 0.00%  | 1.00x     |
| UuidStringConversionBench | benchStringConversionOfHugeUuid                                         | 0   | 5    | 5   | 6,153,928b | 1.000μs     | 1.000μs     | 1.000μs     | 1.000μs     | 0.000μs  | 0.00%  | 1.00x     |
| UuidStringConversionBench | benchStringConversionOfUuid                                             | 0   | 5    | 5   | 6,153,928b | 1.200μs     | 1.200μs     | 1.200μs     | 1.200μs     | 0.000μs  | 0.00%  | 1.20x     |
| UuidStringConversionBench | benchStringConversionOfPromiscuousUuids                                 | 0   | 5    | 5   | 6,153,936b | 110.200μs   | 115.640μs   | 118.770μs   | 119.400μs   | 4.209μs  | 3.64%  | 115.64x   |
| UuidStringConversionBench | benchBytesConversionOfTinyUuid                                          | 0   | 5    | 5   | 6,153,928b | 2.000μs     | 2.000μs     | 2.000μs     | 2.000μs     | 0.000μs  | 0.00%  | 2.00x     |
| UuidStringConversionBench | benchBytesConversionOfHugeUuid                                          | 0   | 5    | 5   | 6,153,928b | 1.800μs     | 1.800μs     | 1.800μs     | 1.800μs     | 0.000μs  | 0.00%  | 1.80x     |
| UuidStringConversionBench | benchBytesConversionOfUuid                                              | 0   | 5    | 5   | 6,153,928b | 1.800μs     | 1.800μs     | 1.800μs     | 1.800μs     | 0.000μs  | 0.00%  | 1.80x     |
| UuidStringConversionBench | benchBytesConversionOfPromiscuousUuids                                  | 0   | 5    | 5   | 6,153,936b | 166.400μs   | 169.360μs   | 168.077μs   | 174.800μs   | 2.899μs  | 1.71%  | 169.36x   |
| UuidFieldExtractionBench  | benchGetFields                                                          | 0   | 5    | 5   | 6,572,536b | 741.200μs   | 769.760μs   | 764.918μs   | 801.800μs   | 20.406μs | 2.65%  | 769.76x   |
| UuidFieldExtractionBench  | benchGetFields10Times                                                   | 0   | 5    | 5   | 6,572,544b | 752.200μs   | 791.080μs   | 802.854μs   | 818.400μs   | 25.143μs | 3.18%  | 791.08x   |
| UuidFieldExtractionBench  | benchGetHex                                                             | 0   | 5    | 5   | 6,583,416b | 791.200μs   | 830.160μs   | 842.167μs   | 859.200μs   | 24.198μs | 2.91%  | 830.16x   |
| UuidFieldExtractionBench  | benchGetHex10Times                                                      | 0   | 5    | 5   | 6,583,424b | 976.600μs   | 1,005.240μs | 992.009μs   | 1,038.600μs | 24.271μs | 2.41%  | 1,005.24x |
| UuidFieldExtractionBench  | benchGetInteger                                                         | 0   | 5    | 5   | 6,917,144b | 1,038.200μs | 1,071.960μs | 1,076.645μs | 1,102.400μs | 21.891μs | 2.04%  | 1,071.96x |
| UuidFieldExtractionBench  | benchGetInteger10Times                                                  | 0   | 5    | 5   | 6,917,152b | 1,335.000μs | 1,369.000μs | 1,358.176μs | 1,420.200μs | 31.531μs | 2.30%  | 1,369.00x |
| UuidGenerationBench       | benchUuid1GenerationWithoutParameters                                   | 0   | 5    | 5   | 6,561,584b | 1,128.400μs | 1,160.200μs | 1,150.947μs | 1,204.200μs | 27.000μs | 2.33%  | 1,160.20x |
| UuidGenerationBench       | benchUuid1GenerationWithNode                                            | 0   | 5    | 5   | 6,561,360b | 1,058.600μs | 1,092.080μs | 1,080.670μs | 1,134.800μs | 27.082μs | 2.48%  | 1,092.08x |
| UuidGenerationBench       | benchUuid1GenerationWithNodeAndClockSequence                            | 0   | 5    | 5   | 6,561,384b | 1,092.000μs | 1,117.040μs | 1,126.834μs | 1,132.000μs | 15.455μs | 1.38%  | 1,117.04x |
| UuidGenerationBench       | benchUuid2GenerationWithDomainAndLocalIdentifier                        | 0   | 5    | 5   | 6,871,792b | 1,623.800μs | 1,646.240μs | 1,633.976μs | 1,688.800μs | 23.964μs | 1.46%  | 1,646.24x |
| UuidGenerationBench       | benchUuid2GenerationWithDomainAndLocalIdentifierAndNode                 | 0   | 5    | 5   | 6,871,792b | 1,309.400μs | 1,335.040μs | 1,345.401μs | 1,354.600μs | 17.389μs | 1.30%  | 1,335.04x |
| UuidGenerationBench       | benchUuid2GenerationWithDomainAndLocalIdentifierAndNodeAndClockSequence | 0   | 5    | 5   | 6,871,808b | 1,325.800μs | 1,355.000μs | 1,344.950μs | 1,396.200μs | 24.257μs | 1.79%  | 1,355.00x |
| UuidGenerationBench       | benchUuid3Generation                                                    | 0   | 5    | 5   | 6,550,032b | 670.000μs   | 680.040μs   | 685.460μs   | 689.800μs   | 8.350μs  | 1.23%  | 680.04x   |
| UuidGenerationBench       | benchUuid4Generation                                                    | 0   | 5    | 5   | 6,550,032b | 641.400μs   | 670.680μs   | 664.213μs   | 701.800μs   | 21.455μs | 3.20%  | 670.68x   |
| UuidGenerationBench       | benchUuid5Generation                                                    | 0   | 5    | 5   | 6,550,080b | 694.200μs   | 722.640μs   | 719.151μs   | 745.200μs   | 19.055μs | 2.64%  | 722.64x   |
| UuidGenerationBench       | benchUuid6GenerationWithoutParameters                                   | 0   | 5    | 5   | 6,561,584b | 1,134.200μs | 1,156.240μs | 1,154.181μs | 1,180.400μs | 15.061μs | 1.30%  | 1,156.24x |
| UuidGenerationBench       | benchUuid6GenerationWithNode                                            | 0   | 5    | 5   | 6,561,360b | 1,078.600μs | 1,115.680μs | 1,100.530μs | 1,161.000μs | 30.601μs | 2.74%  | 1,115.68x |
| UuidGenerationBench       | benchUuid6GenerationWithNodeAndClockSequence                            | 0   | 5    | 5   | 6,561,384b | 1,049.800μs | 1,103.640μs | 1,123.668μs | 1,138.200μs | 33.503μs | 3.04%  | 1,103.64x |
| UuidSerializationBench    | benchSerializationOfTinyUuid                                            | 0   | 5    | 5   | 6,147,304b | 1.600μs     | 1.600μs     | 1.600μs     | 1.600μs     | 0.000μs  | 0.00%  | 1.60x     |
| UuidSerializationBench    | benchSerializationOfHugeUuid                                            | 0   | 5    | 5   | 6,147,304b | 1.600μs     | 1.600μs     | 1.600μs     | 1.600μs     | 0.000μs  | 0.00%  | 1.60x     |
| UuidSerializationBench    | benchSerializationOfUuid                                                | 0   | 5    | 5   | 6,147,304b | 1.600μs     | 1.600μs     | 1.600μs     | 1.600μs     | 0.000μs  | 0.00%  | 1.60x     |
| UuidSerializationBench    | benchSerializationOfPromiscuousUuids                                    | 0   | 5    | 5   | 6,147,312b | 63.800μs    | 64.880μs    | 64.082μs    | 66.800μs    | 1.211μs  | 1.87%  | 64.88x    |
| UuidSerializationBench    | benchDeSerializationOfTinyUuid                                          | 0   | 5    | 5   | 6,147,304b | 2.200μs     | 2.200μs     | 2.200μs     | 2.200μs     | 0.000μs  | 0.00%  | 2.20x     |
| UuidSerializationBench    | benchDeSerializationOfHugeUuid                                          | 0   | 5    | 5   | 6,147,304b | 2.200μs     | 2.200μs     | 2.200μs     | 2.200μs     | 0.000μs  | 0.00%  | 2.20x     |
| UuidSerializationBench    | benchDeSerializationOfUuid                                              | 0   | 5    | 5   | 6,147,304b | 2.200μs     | 2.200μs     | 2.200μs     | 2.200μs     | 0.000μs  | 0.00%  | 2.20x     |
| UuidSerializationBench    | benchDeSerializationOfPromiscuousUuids                                  | 0   | 5    | 5   | 6,147,312b | 85.800μs    | 88.520μs    | 89.896μs    | 90.400μs    | 1.929μs  | 2.18%  | 88.52x    |
+---------------------------+-------------------------------------------------------------------------+-----+------+-----+------------+-------------+-------------+-------------+-------------+----------+--------+-----------+

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Uuid::uuid4() and similar API is still extremely slow.

I don't know if that can be sped up, but it should be another area of focus for 5.x (benchmarks are in place, but I couldn't find an easy-pick source of the problem, not even with XDebug profiling)

Copy link

@pounard pounard Jul 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uuid::uuid4() being slow is far less annoying that fromString() and toString() being slow, we create an UUID for an entity or such only once, we load and update it thousands if not millions of time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pounard yes, but it is still relevant for anything that generates in-request UUIDs, for example tracing/logging use-cases.

Anyway, the patch here is done as-is :)

Ocramius and others added 13 commits July 9, 2020 13:56
…`Uuid#toString()`, `Uuid#getBytes()`

This is mostly to store an initial state, before we go on and start manipulating the UUID library internals.
…ing` when possible

This should speed up `Uuid::fromString()` massively, leading to much shallower execution
paths when `toString()` and similar simplistic API is required.
…chmarks

This is not indicative of performance, but just guarantees that the benchmarks
still run, and that we didn't break them during an upgrade or code change.
…ality guidelines

We have a lot of deprecated API in here which shouldn't exist in first place, but that
will luckily disappear with v5.0.0 :-)
…o `Uuid::fromString()` and `Uuid::fromBytes()`

With this change, `Uuid::uuid1()`, `Uuid::uuid2()` and so forth now produce a `LazyUuidFromString` instance, which
is both more memory efficient and comparable to `Uuid::fromString()` instances in other tools, such as within
PHPUnit's `Assertion::assertEqual()`, which would reject any two objects not matching each other's types.

Before this patch, `Assertion::assertEquals(Uuid::uuid5(...), Uuid::fromString(...))` would always fail due to
different subtypes produced by the two factory methods.
…s an internal instance cache for unwrapped instances
@Ocramius Ocramius force-pushed the feature/happy-path-performance-improvements branch from 441df48 to 0c2b407 Compare July 9, 2020 12:41
@ramsey ramsey merged commit 9ebb0eb into ramsey:master Jul 10, 2020
@Ocramius Ocramius deleted the feature/happy-path-performance-improvements branch July 17, 2020 02:32
@Ocramius Ocramius restored the feature/happy-path-performance-improvements branch July 17, 2020 02:32
@Ocramius Ocramius deleted the feature/happy-path-performance-improvements branch July 17, 2020 02:32
@Ocramius
Copy link
Contributor Author

@ramsey could you try tagging this as something like 4.1.0 when you got some time?

I'd like to see the effects on the wider ecosystem :D

agustingomes added a commit to agustingomes/chimeraphp-foundation that referenced this pull request Jul 30, 2020
The generation of Uuid objects received a substantial improvement
in this PR ramsey/uuid#324 which introduced
the new object being returned from the default UuidFactory.
@shehi
Copy link

shehi commented Aug 17, 2020

Just wanted to note that from version 4.0 to version 4.1, this introduction of Lazy classes has created BC implications. Applications started to break. Tests not passing anymore. Was this expected and/or intended? But otherwise, all in all, a great work!

@Ocramius
Copy link
Contributor Author

@shehi breakages if you didn't respect the types of the various Uuid::*() static methods are to expected: those are not really BC, but rather legit bugs in downstream consumers.

There is no guarantee that a specific UuidInterface implementation will be built: that's part of the API contract

@shehi
Copy link

shehi commented Aug 17, 2020

Apologies @Ocramius . I took over uuid-doctrine-odm package maintenance recently, and that package had wrong type-checking (instead of checking against UuidInterface in many places those checks were against specific implementations of that interface). I posted my above comment after I fixed those and released new version, but somehow I didn't notice my new release had a delay in propagating into packagist (when I did composer update, it actually didn't install the new released version). As a result, when I saw bugs persisting, my first reaction was: "haha, I found BC issues!!!" :D But no, everything is working just fine. Sorry again for false alarm!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants