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

Cycle: disable tokenizer #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

roxblnfk
Copy link
Contributor

@roxblnfk roxblnfk commented Dec 26, 2023

ORM Schema compilation usually doesn't run in a production environment, but is loaded from the cache. This can affect the Memory Peak metric.
Perhaps it would be more correct to set the schema manually, rather than parsing files and attributes. #1 (comment)

Also removed unused packages.

I think the same is fair to do for Doctrine and Compositedb - to take out cache compilation. And in the startup script, use the ready result.
Then the Memory Peak metric will be collected under more equal conditions.

@compositephp
Copy link
Owner

Sorry, but now I'm uncertain about this PR for now. While I understand and respect your intentions to describe the model schema manually, my approach has been to maintain maximum consistency with real user scenarios. I think that majority of CycleORM users would opt for Annotated Entities rather than maintaining the manual schema in two separate places.

Additionally, I prefer not to delve into advanced fine-tuning and caching, as it can lead to unpredictable comparison conditions. I believe everything should adhere as closely as possible to the official basic documentation.

In the current benchmark, every ORM has almost identical model to scan and build it schema.

However, I'll take a closer look at Doctrine to see if it's possible to implement the same optimizations you made for Cycle without activating the caching layer. After this review, I'll be in a better position to consider applying your PR.

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.

2 participants