Closed
Bug 787291
Opened 9 years ago
Closed 9 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•9 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•9 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•9 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•9 years ago
|
||
Thank you Ms2ger https://hg.mozilla.org/integration/mozilla-inbound/rev/949ade2a2ea1
Comment 6•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/949ade2a2ea1
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in
before you can comment on or make changes to this bug.
Description
•