787291 - add pod_malloc<T> et al

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

Attachment: patch

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.

Comment 1

[Luke Wagner [:luke]]

Attachment: patch

Whitespace fix.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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.

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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.

Comment 4

[Luke Wagner [:luke]]

(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.

Comment 5

[Luke Wagner [:luke]]

Thank you Ms2ger
https://hg.mozilla.org/integration/mozilla-inbound/rev/949ade2a2ea1

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/949ade2a2ea1