Closed Bug 664677 Opened 9 years ago Closed 9 years ago

calling InitRuntimeNumberState when instatiating the runtime

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: igor, Assigned: igor)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

Currently js_InitRuntimeNumberState is called when constructing the first context. However, the implementation does not depend on JSContext *cx. As such it should be called when we create an instance of JSRuntime.
Attached patch v1Splinter Review
In addition to the number state initialization the patch also moves script runtime setup to JSRuntime:init.
Attachment #539768 - Flags: review?(anygregor)
Comment on attachment 539768 [details] [diff] [review]
v1

> 
>-    return rt->thousandsSeparator && rt->decimalSeparator && rt->numGrouping;
>+    memcpy(storage, thousandsSeparator, thousandsSeparatorSize);
>+    rt->thousandsSeparator = storage;
>+    storage += thousandsSeparatorSize;
>+
>+    memcpy(storage, decimalPoint, decimalPointSize);
>+    rt->decimalSeparator = storage;
>+    storage += decimalPointSize;
>+
>+    memcpy(storage, grouping, groupingSize);
>+    rt->numGrouping = grouping;
>+    return true;

Please add a short comment here describing your one malloc for 3 strings.

> }
> 
> void
>-js_FinishRuntimeNumberState(JSContext *cx)
>+FinishRuntimeNumberState(JSRuntime *rt)
> {
>-    JSRuntime *rt = cx->runtime;
>+    Foreground::free_(const_cast<char *>(rt->thousandsSeparator));
>+}
> 
>-    cx->free_((void *) rt->thousandsSeparator);
>-    cx->free_((void *) rt->decimalSeparator);
>-    cx->free_((void *) rt->numGrouping);
>-    rt->thousandsSeparator = rt->decimalSeparator = rt->numGrouping = NULL;
>-}
>+} /* namespace js */
> 


Here as well, explain that one free is enough.
Attachment #539768 - Flags: review?(anygregor) → review+
http://hg.mozilla.org/tracemonkey/rev/cd2baff53251 - pushed with an extra comments
Whiteboard: fixed-in-tracemonkey
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.