[webkit-dev] Always-on diagnostic code Re: [webkit-changes] [96819] trunk/Source/WebCore
On Oct 6, 2011, at 9:40 AM, gav...@chromium.org wrote:
Modified: trunk/Source/WebCore/dom/ScriptElement.h (96818 = 96819)
--- trunk/Source/WebCore/dom/ScriptElement.h 2011-10-06 16:37:35 UTC (rev
96818)
+++ trunk/Source/WebCore/dom/ScriptElement.h 2011-10-06 16:40:47 UTC (rev
96819)
@@ -113,6 +113,14 @@
ZeroedInStopLoadRequest,
ZeroedInNotifyFinished,
} m_cachedScriptState;
+
+// We grab a backtrace when we zero m_cachedScript, so that at later
crashes
+// we'll have a debuggable stack.
+enum {
+MaxBacktraceSize = 32
+};
+int m_backtraceSize;
+void* m_backtrace[MaxBacktraceSize];
};
This appears to increase the size of each ScriptElement instance by 256 bytes.
I don’t know how bad a performance hit this is in real-world use, but it is
most certainly not something all vendors would like to include in their
releases. The way this change was made, however, it is almost inevitable that a
vendor would end up unknowingly shipping this performance regression. This
change was made on trunk, it is unconditionally compiled in, and there is
nothing obvious tracking undoing this change.
I think this is the wrong way to incorporate diagnostic code into WebKit.___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Always-on diagnostic code Re: [webkit-changes] [96819] trunk/Source/WebCore
Dan,
You're right. I'm adding #if PLATFORM(CHROMIUM) to this change shortly.
- Gavin
On 6 October 2011 13:07, Dan Bernstein m...@apple.com wrote:
On Oct 6, 2011, at 9:40 AM, gav...@chromium.org wrote:
Modified: trunk/Source/WebCore/dom/ScriptElement.h (96818 = 96819)
--- trunk/Source/WebCore/dom/ScriptElement.h 2011-10-06 16:37:35 UTC (rev
96818)
+++ trunk/Source/WebCore/dom/ScriptElement.h 2011-10-06 16:40:47 UTC (rev
96819)@@ -113,6 +113,14 @@ ZeroedInStopLoadRequest,
ZeroedInNotifyFinished, } m_cachedScriptState;+
+// We grab a backtrace when we zero m_cachedScript, so that at later
crashes
+// we'll have a debuggable stack.
+enum {
+MaxBacktraceSize = 32
+};
+int m_backtraceSize;
+void* m_backtrace[MaxBacktraceSize]; };
This appears to increase the size of each ScriptElement instance by 256
bytes. I don’t know how bad a performance hit this is in real-world use, but
it is most certainly not something all vendors would like to include in
their releases. The way this change was made, however, it is almost
inevitable that a vendor would end up unknowingly shipping this performance
regression. This change was made on trunk, it is unconditionally compiled
in, and there is nothing obvious tracking undoing this change.
I think this is the wrong way to incorporate diagnostic code into WebKit.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Always-on diagnostic code Re: [webkit-changes] [96819] trunk/Source/WebCore
I agree that this seems like something to #ifdef. Out of curiosity, did you
just stumble upon this, or did you actually notice a measurable performance
regression from this?
-Darin
On Thu, Oct 6, 2011 at 10:07 AM, Dan Bernstein m...@apple.com wrote:
On Oct 6, 2011, at 9:40 AM, gav...@chromium.org wrote:
Modified: trunk/Source/WebCore/dom/ScriptElement.h (96818 = 96819)
--- trunk/Source/WebCore/dom/ScriptElement.h 2011-10-06 16:37:35 UTC (rev
96818)
+++ trunk/Source/WebCore/dom/ScriptElement.h 2011-10-06 16:40:47 UTC (rev
96819)@@ -113,6 +113,14 @@ ZeroedInStopLoadRequest,
ZeroedInNotifyFinished, } m_cachedScriptState;+
+// We grab a backtrace when we zero m_cachedScript, so that at later
crashes
+// we'll have a debuggable stack.
+enum {
+MaxBacktraceSize = 32
+};
+int m_backtraceSize;
+void* m_backtrace[MaxBacktraceSize]; };
This appears to increase the size of each ScriptElement instance by 256
bytes. I don’t know how bad a performance hit this is in real-world use, but
it is most certainly not something all vendors would like to include in
their releases. The way this change was made, however, it is almost
inevitable that a vendor would end up unknowingly shipping this performance
regression. This change was made on trunk, it is unconditionally compiled
in, and there is nothing obvious tracking undoing this change.
I think this is the wrong way to incorporate diagnostic code into WebKit.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Always-on diagnostic code Re: [webkit-changes] [96819] trunk/Source/WebCore
On Oct 6, 2011, at 10:12 AM, Darin Fisher wrote:
I agree that this seems like something to #ifdef. Out of curiosity, did you
just stumble upon this, or did you actually notice a measurable performance
regression from this?
I just noticed the patch go by.
-Darin
On Thu, Oct 6, 2011 at 10:07 AM, Dan Bernstein m...@apple.com wrote:
On Oct 6, 2011, at 9:40 AM, gav...@chromium.org wrote:
Modified: trunk/Source/WebCore/dom/ScriptElement.h (96818 = 96819)
--- trunk/Source/WebCore/dom/ScriptElement.h 2011-10-06 16:37:35 UTC (rev
96818)
+++ trunk/Source/WebCore/dom/ScriptElement.h 2011-10-06 16:40:47 UTC (rev
96819)
@@ -113,6 +113,14 @@
ZeroedInStopLoadRequest,
ZeroedInNotifyFinished,
} m_cachedScriptState;
+
+// We grab a backtrace when we zero m_cachedScript, so that at later
crashes
+// we'll have a debuggable stack.
+enum {
+MaxBacktraceSize = 32
+};
+int m_backtraceSize;
+void* m_backtrace[MaxBacktraceSize];
};
This appears to increase the size of each ScriptElement instance by 256
bytes. I don’t know how bad a performance hit this is in real-world use, but
it is most certainly not something all vendors would like to include in their
releases. The way this change was made, however, it is almost inevitable that
a vendor would end up unknowingly shipping this performance regression. This
change was made on trunk, it is unconditionally compiled in, and there is
nothing obvious tracking undoing this change.
I think this is the wrong way to incorporate diagnostic code into WebKit.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Always-on diagnostic code Re: [webkit-changes] [96819] trunk/Source/WebCore
On Oct 6, 2011, at 10:11 AM, Gavin Peters (蓋文彼德斯) wrote:
Dan,
You're right. I'm adding #if PLATFORM(CHROMIUM) to this change shortly.
Thank you.
- Gavin
On 6 October 2011 13:07, Dan Bernstein m...@apple.com wrote:
On Oct 6, 2011, at 9:40 AM, gav...@chromium.org wrote:
Modified: trunk/Source/WebCore/dom/ScriptElement.h (96818 = 96819)
--- trunk/Source/WebCore/dom/ScriptElement.h 2011-10-06 16:37:35 UTC (rev
96818)
+++ trunk/Source/WebCore/dom/ScriptElement.h 2011-10-06 16:40:47 UTC (rev
96819)
@@ -113,6 +113,14 @@
ZeroedInStopLoadRequest,
ZeroedInNotifyFinished,
} m_cachedScriptState;
+
+// We grab a backtrace when we zero m_cachedScript, so that at later
crashes
+// we'll have a debuggable stack.
+enum {
+MaxBacktraceSize = 32
+};
+int m_backtraceSize;
+void* m_backtrace[MaxBacktraceSize];
};
This appears to increase the size of each ScriptElement instance by 256
bytes. I don’t know how bad a performance hit this is in real-world use, but
it is most certainly not something all vendors would like to include in their
releases. The way this change was made, however, it is almost inevitable that
a vendor would end up unknowingly shipping this performance regression. This
change was made on trunk, it is unconditionally compiled in, and there is
nothing obvious tracking undoing this change.
I think this is the wrong way to incorporate diagnostic code into WebKit.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
