fix template class name generation to prevent possible collisions#1852
fix template class name generation to prevent possible collisions#1852fabpot merged 1 commit intotwigphp:1.xfrom Tobion:fix-possible-class-conflicts
Conversation
Tobion
commented
Sep 24, 2015
Tobion
commented
- fix template class name generation to prevent possible collisions
- save one inode per file
- fix tests as they are broken with the cext since changed template cache names to take into account the Twig C extension #1844
There was a problem hiding this comment.
using one directory instead of two saves 1 inode per file. the file distribution stays the same. even though the math in #1684 is wrong, I agree that this diversity is enough.
There was a problem hiding this comment.
It was done with a hash because of your comment here: #1822 (comment)
👎 for this change. The getTemplateClass() is public and can be changed in any way.
There was a problem hiding this comment.
I know. I'm just trying to make it as fast as possible by default and avoid hashing the hash for every template on each request. Maybe we should remove the directory balancing at all by default. Todays filesystems do not have a limitation on number of files per directory. And also most users, won't have thousands of templates anyway where it could matter. And if people really have this need they can change the filesystem cache themselves. No need to hurt everybody by default.
There was a problem hiding this comment.
This was added because some of our customers had this problem.
There was a problem hiding this comment.
Yeah but IMO performance-relevant things that only affect a small subset of people should be opt-in and not opt-out. Even more so now that the caching implementation can easily be adapted.
There was a problem hiding this comment.
No, Twig should work for everyone and it is the case right now.
There was a problem hiding this comment.
reverted even though I don't fully agree. It can never work for everyone as it depends on the filesystem and the number of templates you have. So whatever workaround you add it can always be the wrong one depending on who is using it.
Tobion
changed the title
… one inode per file
Tobion
changed the title
|
Thank you @Tobion. |
…isions (Tobion) This PR was merged into the 1.x branch. Discussion ---------- fix template class name generation to prevent possible collisions - fix template class name generation to prevent possible collisions - save one inode per file - fix tests as they are broken with the cext since #1844 Commits ------- f379e81 fix template class name generation to prevent possible collisions and one inode per file
6 participants
