WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
53592
Web Inspector: Add reporting of JS heap size limit to 'console.memory'
https://bugs.webkit.org/show_bug.cgi?id=53592
Summary
Web Inspector: Add reporting of JS heap size limit to 'console.memory'
Mikhail Naganov
Reported
2011-02-02 06:53:54 PST
This helps to evaluate memory impact of a web application. Also it's good to know maximum available VM memory size, as it differs between browsers, and their versions (e.g. 32-bit vs. 64-bit).
Attachments
patch
(10.92 KB, patch)
2011-02-02 09:14 PST
,
Mikhail Naganov
mnaganov
: commit-queue-
Details
Formatted Diff
Diff
fixed style
(10.83 KB, patch)
2011-02-02 09:28 PST
,
Mikhail Naganov
mnaganov
: commit-queue-
Details
Formatted Diff
Diff
return 'undefined' limit value for JSC
(16.84 KB, patch)
2011-02-03 01:49 PST
,
Mikhail Naganov
mnaganov
: commit-queue-
Details
Formatted Diff
Diff
fixed custom getter signature
(16.78 KB, patch)
2011-02-03 04:17 PST
,
Mikhail Naganov
pfeldman
: review+
mnaganov
: commit-queue-
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Patrick Mueller
Comment 1
2011-02-02 07:12:12 PST
Any security aspects here? Seems like I remember there was something similar that was pulled from console last year, due to security concerns. I'd like to see this implemented though.
Mikhail Naganov
Comment 2
2011-02-02 07:15:23 PST
After discussions, we made memory info available only by explicit user request (preference in Safari, cmdline flag in Chrome). This resolves security concerns, I believe.
Mikhail Naganov
Comment 3
2011-02-02 09:14:27 PST
Created
attachment 80919
[details]
patch @JSC developers: The MAX_NUM_BLOCKS const (ex. maxNumBlocks) appears to be ridiculously big -- being multiplied by BLOCK_SIZE it exceeds ULONG_MAX. Does it need to be adjusted somehow? On a 64-bit platform MAX_NUM_BLOCKS value is 2^59 - 1 (as sizeof(PageAllocationAligned) = 16), and BLOCK_SIZE is 2^18.
WebKit Review Bot
Comment 4
2011-02-02 09:17:06 PST
Attachment 80919
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Source/JavaScriptCore/runtime/MarkedSpace.cpp:43: MAX_NUM_BLOCKS is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Total errors found: 1 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Mikhail Naganov
Comment 5
2011-02-02 09:28:48 PST
Created
attachment 80921
[details]
fixed style Renamed the constant back, as the new name introduces style violation.
Build Bot
Comment 6
2011-02-02 09:42:52 PST
Attachment 80919
[details]
did not build on win: Build output:
http://queues.webkit.org/results/7683985
Mikhail Naganov
Comment 7
2011-02-02 09:50:12 PST
...and VC++ confirms that maxNumBlocks is uselessly big. Geoffrey, can you comment on this, please?
Build Bot
Comment 8
2011-02-02 10:06:27 PST
Attachment 80921
[details]
did not build on win: Build output:
http://queues.webkit.org/results/7689214
Geoffrey Garen
Comment 9
2011-02-02 10:57:36 PST
What does "limit" mean in this context? JavaScriptCore doesn't place a hard limit on the size of the heap -- it will use whatever the OS will give it.
Mikhail Naganov
Comment 10
2011-02-02 11:00:34 PST
But there can be other restrictions that prevent VM from allocating new space above a certain limit, e.g. control structures size limits. Why this maxNumBlocks constant exist at all?
Geoffrey Garen
Comment 11
2011-02-02 11:08:11 PST
> Why this maxNumBlocks constant exist at all?
It seems to exist solely to check for overflow in multiplication. Whatever you're trying to do, this probably isn't the best way to do it. What does "limit" mean in this context? What are you trying to do?
Mikhail Naganov
Comment 12
2011-02-02 13:05:59 PST
I introduced this change because in V8 there is actually a hard limit on the VM heap size, and its value differs on 32-bit and 64-bit versions, so I didn't want to hardcode it. But knowing the limit appears to be useful, because when web app memory consumption grows up to the limit, renderer crashes. If JSC really doesn't have a limit, I can put something like undefined in this field. I wanted to make sure that both engines are aligned.
Geoffrey Garen
Comment 13
2011-02-02 16:36:26 PST
undefined sounds good to me.
Mikhail Naganov
Comment 14
2011-02-03 01:49:39 PST
Created
attachment 81043
[details]
return 'undefined' limit value for JSC
Build Bot
Comment 15
2011-02-03 02:51:05 PST
Attachment 81043
[details]
did not build on win: Build output:
http://queues.webkit.org/results/7692258
Early Warning System Bot
Comment 16
2011-02-03 03:29:54 PST
Attachment 81043
[details]
did not build on qt: Build output:
http://queues.webkit.org/results/7687838
Mikhail Naganov
Comment 17
2011-02-03 04:17:37 PST
Created
attachment 81049
[details]
fixed custom getter signature
Mikhail Naganov
Comment 18
2011-02-03 08:34:25 PST
Manually committed
http://trac.webkit.org/changeset/77495
2011-02-03 Mikhail Naganov <
mnaganov@chromium.org
> Reviewed by Pavel Feldman. Web Inspector: Add reporting of JS heap size limit to 'console.memory'.
https://bugs.webkit.org/show_bug.cgi?id=53592
In JSC there is no limit, thus 'undefined' value is returned. For V8, the limit reported by the VM is returned. * Android.jscbindings.mk: * CMakeLists.txt: * GNUmakefile.am: * WebCore.gypi: * WebCore.pro: * WebCore.vcproj/WebCore.vcproj: * WebCore.xcodeproj/project.pbxproj: * bindings/js/JSBindingsAllInOne.cpp: * bindings/js/JSMemoryInfoCustom.cpp: Added. * bindings/js/ScriptGCEvent.cpp: (WebCore::ScriptGCEvent::getHeapSize): * bindings/js/ScriptGCEvent.h: * bindings/v8/ScriptGCEvent.cpp: (WebCore::ScriptGCEvent::getHeapSize): * bindings/v8/ScriptGCEvent.h: * inspector/InspectorTimelineAgent.cpp: (WebCore::InspectorTimelineAgent::setHeapSizeStatistic): * page/MemoryInfo.cpp: (WebCore::MemoryInfo::MemoryInfo): * page/MemoryInfo.h: (WebCore::MemoryInfo::jsHeapSizeLimit): * page/MemoryInfo.idl:
Adam Barth
Comment 19
2011-02-03 10:48:08 PST
> Manually committed
http://trac.webkit.org/changeset/77495
This broke a test on Snow Leopard:
http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r77508%20(24728)/fast/dom/Window/window-properties-pretty-diff.html
WebKit Review Bot
Comment 20
2011-02-03 12:06:03 PST
http://trac.webkit.org/changeset/77495
might have broken GTK Linux 32-bit Debug
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug