improve StyleRareNonInheritedData bit packing on Windows
Created attachment 143678 [details] Patch
I didn't add a compile assert because it's OK for this struct to grow slowly.
Comment on attachment 143678 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143678&action=review LGTM. > Source/WebCore/rendering/style/StyleRareNonInheritedData.cpp:150 > bool StyleRareNonInheritedData::operator==(const StyleRareNonInheritedData& o) const Presumably we could optimize the comparison order. Do you believe that was attempted previously? I'm OK with this kept in this order if you think it's unlikely to matter for perf.
Comment on attachment 143678 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143678&action=review > Source/WebCore/rendering/style/StyleRareNonInheritedData.h:176 > - bool m_hasAspectRatio : 1; // Whether or not an aspect ratio has been specified. > #if USE(ACCELERATED_COMPOSITING) > - bool m_runningAcceleratedAnimation : 1; > + unsigned m_runningAcceleratedAnimation : 1; > #endif > + unsigned m_hasAspectRatio : 1; // Whether or not an aspect ratio has been specified. Could you please add a compile assertion?
Comment on attachment 143678 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143678&action=review >> Source/WebCore/rendering/style/StyleRareNonInheritedData.cpp:150 >> bool StyleRareNonInheritedData::operator==(const StyleRareNonInheritedData& o) const > > Presumably we could optimize the comparison order. Do you believe that was attempted previously? I'm OK with this kept in this order if you think it's unlikely to matter for perf. I don't think perf was a concern. Looking at the history of the header file, members have been reordered multiple times for bit packing reasons. I bet it was just an oversight that this wasn't reordered. >> Source/WebCore/rendering/style/StyleRareNonInheritedData.h:176 >> + unsigned m_hasAspectRatio : 1; // Whether or not an aspect ratio has been specified. > > Could you please add a compile assertion? Do you think it's worth it given that this object can grow? It's already 448 bytes on my 64 bit machine.
(In reply to comment #5) > >> Source/WebCore/rendering/style/StyleRareNonInheritedData.h:176 > >> + unsigned m_hasAspectRatio : 1; // Whether or not an aspect ratio has been specified. > > > > Could you please add a compile assertion? > > Do you think it's worth it given that this object can grow? It's already 448 bytes on my 64 bit machine. Oh, I see. Maybe not then.
Comment on attachment 143678 [details] Patch Clearing flags on attachment: 143678 Committed r118396: <http://trac.webkit.org/changeset/118396>
All reviewed patches have been landed. Closing bug.