Closed Bug 787291 Opened 13 years ago Closed 13 years ago

add pod_malloc<T> et al

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: luke, Assigned: luke)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch patch (obsolete) — 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)
Attached patch patchSplinter Review
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 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.
(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.
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.

Attachment

General

Created:
Updated:
Size: