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.

Sign up for GitHub

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

Jump to bottom

Merge 2.19.x into 3.1.x #11392

Merged
8 commits merged into from
Mar 21, 2024
Merged

Merge 2.19.x into 3.1.x #11392

8 commits merged into from
Mar 21, 2024

Conversation

derrabus
Copy link
Member

No description provided.

beberlei and others added 5 commits March 19, 2024 09:19
@beberlei @nicolas-grekas
Improve lazy ghost performance by avoiding self-referencing closure. (d…
9d1a497 
…octrine#11376)

* Improve lazy ghost performance by avoiding self-referencing closure.

Co-authored-by: Nicolas Grekas <[email protected]>

* update baselien

---------

Co-authored-by: Nicolas Grekas <[email protected]>
@beberlei
[doctrineGH-11185] Bugfix: do not use collection batch loading for in…
d15624f 
…dexBy assocations. (doctrine#11380)
@ThomasLandauer
[Documentation] Removing "Doctrine Mapping Types" ... (doctrine#11384)
5ccbc20 
... in favor of https://www.doctrine-project.org/projects/doctrine-dbal/en/3.8/reference/types.html#reference

Page: https://www.doctrine-project.org/projects/doctrine-orm/en/2.19/reference/basic-mapping.html#doctrine-mapping-types

As announced in doctrine/dbal#6336 (comment) , the goal is to remove this duplicated type information from ORM and replace it with a link to DBAL.

In doctrine/dbal#6341 , I'm adding any detail which I'm deleting here to the DBAL.
@derrabus
Remove unused variable (doctrine#11391)
db6e702
@derrabus
Add missing import
95795c8
@derrabus derrabus force-pushed the 3.1.x branch 4 times, most recently from 34a410e to 4ebc6c0 Compare March 21, 2024 10:17
@derrabus
Copy link
Member Author

derrabus commented Mar 21, 2024 •
edited
Loading

@greg0ire @beberlei I need help with the merge-up.

derrabus
derrabus commented Mar 21, 2024
View reviewed changes
composer.json
@@ -33,7 +33,7 @@
"doctrine/persistence": "^3.3.1",
"psr/cache": "^1 || ^2 || ^3",
"symfony/console": "^5.4 || ^6.0 || ^7.0",
"symfony/var-exporter": "~6.2.13 || ^6.3.2 || ^7.0"
"symfony/var-exporter": "^6.3.9 || ^7.0"
Copy link
Member Author

Choose a reason for hiding this comment

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

I had to bump here to make our low-deps run pass. Apparently after #11376, we were hitting a bug in VarExporter that had been fixed in 6.3.9/6.4.0/7.0.0. I believe this bump is acceptable for a bugfix.

derrabus added 2 commits March 21, 2024 12:01
@derrabus
Set column length explicitly (doctrine#11393)
1a5a4c6
@derrabus
Merge branch '2.19.x' into 3.1.x
0a1988b 
* 2.19.x:
  Set column length explicitly (doctrine#11393)
  Add missing import
  Remove unused variable (doctrine#11391)
  [Documentation] Removing "Doctrine Mapping Types" ... (doctrine#11384)
  [doctrineGH-11185] Bugfix: do not use collection batch loading for indexBy assocations. (doctrine#11380)
  Improve lazy ghost performance by avoiding self-referencing closure. (doctrine#11376)
@greg0ire
Copy link
Member

greg0ire commented Mar 21, 2024 •
edited
Loading

@derrabus @beberlei on 2.x, phpbench runs on PHP 7.4. I reproduce the issue locally on 2.x, with PHP 8.3
The benchmark works by calling a method that sounds like it is internal:

orm/tests/Performance/LazyLoading/ProxyInitializationTimeBench.php

Line 39 in 4175edf

$this->initializedUsers[$i]->__load();

So maybe this issue does not affect end users at all.

@derrabus
Copy link
Member Author

So maybe this issue does not affect end users at all.

It doesn't. The bench installs mock implementations of UoW and friends to prevent the load() calls from actually hitting a database. Apparently, these mocks were incomplete and need to be adjusted to the changes from #11376.

@derrabus
Copy link
Member Author

I've pushed a commit that fixes the mocks. Shall I backport the change and make PHPBench on 2.19 run on PHP 8.1+ with lazy ghosts enabled? Or shall we leave the lower branch as it is now?

@derrabus derrabus requested a review from greg0ire March 21, 2024 12:04
@greg0ire
Copy link
Member

I think it's a good idea to avoid such surprises on merge up so yes, please do that

@derrabus derrabus closed this pull request by merging all changes into doctrine:3.1.x in 9c56071 Mar 21, 2024
@derrabus derrabus deleted the 3.1.x branch March 21, 2024 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greg0ire greg0ire greg0ire approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@derrabus @greg0ire @beberlei @ThomasLandauer