Closed
Bug 787291
Opened 13 years ago
Closed 13 years ago
add pod_malloc<T> et al
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla18
People
(Reporter: luke, Assigned: luke)
References
Details
Attachments
(1 file, 1 obsolete file)
57.81 KB,
patch
|
billm
:
review+
|
Details | Diff | Splinter Review |
This patch lets you change:
T *t = (T *)cx->malloc_(sizeof(T));
T *ts = (T *)cx->malloc_(numElems * sizeof(T));
to:
T *t = cx->pod_alloc<T>();
T *ts = cx->pod_alloc<T>(numElems);
It also adds a similar pod_calloc and global non-accounting js_pod_malloc/js_pod_calloc.
In addition to avoiding all the noisy syntax and increasing type safety, the numElems-taking overloads guard against overflow when multiplying by sizeof(T) (which, using template magic, is a single bit test).
Going through all calls to malloc_/calloc_/js_malloc/js_calloc, the vast majority can be rewritten to use this new pod_* variants.
Attachment #657105 -
Flags: review?(wmccloskey)
![]() |
Assignee | |
Comment 1•13 years ago
|
||
Whitespace fix.
Attachment #657105 -
Attachment is obsolete: true
Attachment #657105 -
Flags: review?(wmccloskey)
Attachment #657108 -
Flags: review?(wmccloskey)
Comment on attachment 657108 [details] [diff] [review]
patch
Review of attachment 657108 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/frontend/BytecodeEmitter.cpp
@@ +2369,5 @@
> intmap_bitlen = INTMAP_LENGTH << JS_BITS_PER_WORD_LOG2;
> } else {
> /* Just grab 8K for the worst-case bitmap. */
> intmap_bitlen = JS_BIT(16);
> + intmap = cx->pod_malloc<jsbitmap>((JS_BIT(16) >> JS_BITS_PER_WORD_LOG2));
Eliminate double parens.
::: js/src/jscntxt.h
@@ +933,5 @@
> }
>
> + template <class T>
> + T *pod_malloc(JSContext *cx = NULL) {
> + return (T *)malloc_(sizeof(T));
Don't you want to pass in cx here and below?
@@ +1435,5 @@
> inline void* realloc_(void* p, size_t oldBytes, size_t newBytes) {
> return runtime->realloc_(p, oldBytes, newBytes, this);
> }
>
> + template <class T> T *pod_malloc(JSContext *cx = NULL) {
I don't think this needs a cx argument. Or the one below.
Attachment #657108 -
Flags: review?(wmccloskey) → review+
Comment 3•13 years ago
|
||
Comment on attachment 657108 [details] [diff] [review]
patch
Review of attachment 657108 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/public/Utility.h
@@ +21,5 @@
>
> #include "jstypes.h"
>
> #ifdef __cplusplus
> +# include "TemplateLib.h"
"js/TemplateLib.h", I think
::: js/src/frontend/BytecodeEmitter.cpp
@@ -2516,5 @@
> * Use malloc to avoid arena bloat for programs with many switches.
> * ScopedFreePtr takes care of freeing it on exit.
> */
> if (tableLength != 0) {
> - tableSize = (size_t)tableLength * sizeof *table;
Please eliminate the declaration too
::: js/src/jsdbgapi.cpp
@@ +360,4 @@
> if (!lines)
> return JS_FALSE;
>
> + pcs = cx->pod_malloc<jsbytecode*>(len);
Bonus points for moving the declarations of those variables down a bit.
![]() |
Assignee | |
Comment 4•13 years ago
|
||
(In reply to Bill McCloskey (:billm) from comment #2)
> Don't you want to pass in cx here and below?
...
> I don't think this needs a cx argument. Or the one below.
Sheesh, yes, good catches. Multitasking is bad for patches.
![]() |
Assignee | |
Comment 5•13 years ago
|
||
Thank you Ms2ger
https://hg.mozilla.org/integration/mozilla-inbound/rev/949ade2a2ea1
Comment 6•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in
before you can comment on or make changes to this bug.
Description
•