Summary: | CTLine objects should outlive their CTRuns | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ned Holbrook <ned> | ||||||
Component: | Text | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, mitz | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.6 | ||||||||
Attachments: |
|
Description
Ned Holbrook
2011-04-07 11:02:18 PDT
The latter approach is less desirable since it would eliminate the possibility of using the CTRunGet*Ptr() accessors, so I'll work up some numbers for the former. Created attachment 88732 [details]
Proposed changes.
Numbers are a bit variable on my machine, but I'm seeing an improvement of around 2% for repeated optimizeLegibility test runs on Latin text—barely above the level of noise but certainly not a slowdown.
Comment on attachment 88732 [details]
Proposed changes.
If the CTLine stays alive, then maybe ComplexTextRun::m_coreTextRun no longer needs to be a RetainPtr?
(In reply to comment #3) > (From update of attachment 88732 [details]) > If the CTLine stays alive, then maybe ComplexTextRun::m_coreTextRun no longer needs to be a RetainPtr? Good idea! Should it become a different smart pointer or will a bare CTRunRef suffice? (In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 88732 [details] [details]) > > If the CTLine stays alive, then maybe ComplexTextRun::m_coreTextRun no longer needs to be a RetainPtr? > > Good idea! Should it become a different smart pointer or will a bare CTRunRef suffice? Bare CTRunRef. A comment right above the member variable definition explaining why it’s ok to use a bare CTRunRef would be good. (In reply to comment #5) > Bare CTRunRef. A comment right above the member variable definition explaining why it’s ok to use a bare CTRunRef would be good. Theoretically it could be removed entirely since it's only used in the constructor. (In reply to comment #6) > (In reply to comment #5) > > Bare CTRunRef. A comment right above the member variable definition explaining why it’s ok to use a bare CTRunRef would be good. > > Theoretically it could be removed entirely since it's only used in the constructor. Let’s remove it in practice, then. Created attachment 88743 [details]
Remove m_coreTextRun.
Comment on attachment 88743 [details] Remove m_coreTextRun. Clearing flags on attachment: 88743 Committed r83251: <http://trac.webkit.org/changeset/83251> All reviewed patches have been landed. Closing bug. |