Closed
Bug 401254
Opened 17 years ago
Closed 17 years ago
jsshell: wrapper for JS_SetGCParameter
Categories
(Core :: JavaScript Engine, enhancement)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: igor, Assigned: igor)
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
igor
:
review+
|
Details | Diff | Splinter Review |
For measuring performance of the GC it would be nice to be able to call from a script JS_SetGCParameter to change JSGC_MAX_BYTES and JSGC_MAX_MALLOC_BYTES as suitable.
Assignee | ||
Comment 1•17 years ago
|
||
The patch adds gcparam to jsshell, a wrapper for JS_SetGCParameter.
Attachment #286297 -
Flags: review?
Assignee | ||
Comment 2•17 years ago
|
||
The next time I should remember that for fast natives the return value must be set in any case.
Attachment #286297 -
Attachment is obsolete: true
Attachment #286300 -
Flags: review?(brendan)
Attachment #286297 -
Flags: review?
Comment 3•17 years ago
|
||
Comment on attachment 286300 [details] [diff] [review]
v2
>+ fprintf(gErrFile,
>+ "gcparam: argument 1 must be either "
>+ "maxBytes or maxMallocBytes\n");
This should be a JS_ReportError call -- also it should imitate other such calls by not including "gcparam: " at the front. Final nit: "first argument" is better than argument 1.
>+ fprintf(gErrFile,
>+ "gcparam: argument 2 must be convertable to uint32 with "
>+ "non-zero value\n");
Ditto.
r=me with these changes.
/be
Attachment #286300 -
Flags: review?(brendan) → review+
Assignee | ||
Comment 4•17 years ago
|
||
Here is the patch to commit with nits addressed. I also replaced fprintf by JS_ReportError in the implementation of countHeap and dumpHeap shell functions.
Attachment #286300 -
Attachment is obsolete: true
Attachment #286437 -
Flags: review+
Assignee | ||
Comment 5•17 years ago
|
||
I checked in the patch from comment 4 to CVS trunk:
Checking in js.c;
/cvsroot/mozilla/js/src/js.c,v <-- js.c
new revision: 3.169; previous revision: 3.168
done
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment 6•17 years ago
|
||
peterv backed this out on 20071028. Shouldn't we reopen it?
Comment 7•17 years ago
|
||
Bah, that's not what I meant to do. I'll reland this tomorrow.
Comment 8•17 years ago
|
||
Undid my backout and relanded this.
Comment 9•17 years ago
|
||
do you think it would be worth it to expose this to the command line js and jsDriver so we could run the test suite with specific values?
Flags: in-testsuite-
Flags: in-litmus-
Assignee | ||
Comment 10•17 years ago
|
||
(In reply to comment #9)
> do you think it would be worth it to expose this to the command line js and
> jsDriver so we could run the test suite with specific values?
The function is already exposed through -e option as in:
js -e 'gcparam("maxBytes", 100<<10)' <other options and arguments>
Theoretically it is not ideal as it executes JS code which may be affected by other options. But I suggest to worry about it when we get a test case where -e would not be enough.
You need to log in
before you can comment on or make changes to this bug.
Description
•