413744 - Using JS_GCMETER without recompiling the browser

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Igor Bukanov]

Currently to enable collecting of GC-stats with the JS_GCMETER macro it is necessary to recompile the browser. The macro adds to the JSGCArenaList struct the extra statistics-related fields. This requires to recompile any code that refer to fields of JSRuntime that comes after JSRuntime.gcArenaList of type JSGCArenaList[GC_NUM_FREELISTS]. That includes xpconnect as it refers, for example, to JSRuntime.gcLock.

It would be nice to move all fields related to GC-stats towards the end of JSRuntime so recompiling just SpiderMonkey's DLL would be sufficient to see JS_GCMETER in action.

Comment 1

[Igor Bukanov]

Attachment: v1

The patch moves JSGCArenaStats from JSGCArenaList into JSGCArenaStats and then moves JSRuntime.gcStats towards the end of JSRuntime.

The patch also adds METER_IF and METER_UPDATE_MAX macros to simplify the metering code.

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 298793 [details] [diff] [review]
v1

Comment about gcStats staying at end of JSRuntime, and r=me.

/be

Comment 3

[Igor Bukanov]

Attachment: v1b

The new version adds the following comments:

--- /home/igor/m/ff/mozilla/quilt.G13159/js/src/jscntxt.h	2008-01-28 18:25:54.000000000 -0800
+++ js/src/jscntxt.h	2008-01-28 18:23:21.000000000 -0800
@@ -374,16 +374,22 @@ struct JSRuntime {
     JSGSNCache          gsnCache;
 
 #define JS_GSN_CACHE(cx) ((cx)->runtime->gsnCache)
 #endif
 
     /* Literal table maintained by jsatom.c functions. */
     JSAtomState         atomState;
 
+    /*
+     * Various metering fields are defined at the end of JSContext. In this
+     * way there is no need to recompile all the code that refers to other
+     * fields of JSContext after enabling the corresponding metering macro.
+     */
+
 #if defined DEBUG || defined JS_DUMP_PROPTREE_STATS
     /* Function invocation metering. */
     jsrefcount          inlineCalls;
     jsrefcount          nativeCalls;
     jsrefcount          nonInlineCalls;
     jsrefcount          constructs;
 
     /* Scope lock and property metering. */

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 299906 [details] [diff] [review]
v1b

>@@ -378,16 +374,22 @@ struct JSRuntime {
>     JSGSNCache          gsnCache;
> 
> #define JS_GSN_CACHE(cx) ((cx)->runtime->gsnCache)
> #endif
> 
>     /* Literal table maintained by jsatom.c functions. */
>     JSAtomState         atomState;
> 
>+    /*
>+     * Various metering fields are defined at the end of JSContext. In this
>+     * way there is no need to recompile all the code that refers to other
>+     * fields of JSContext after enabling the corresponding metering macro.
>+     */

First line shows that the struct these members lie within is not JSContext. r/a=me on comment fix as needed.

/be

Comment 5

[Igor Bukanov]

Attachment: v1c

Here is a patch with the comment fix.

Comment 6

[Igor Bukanov]

Attachment: v1d

Here is patch with one more style nit: 

--- /home/igor/m/ff/mozilla/quilt.y19665/js/src/jsgc.c	2008-01-29 11:22:01.000000000 -0800
+++ js/src/jsgc.c	2008-01-29 11:21:43.000000000 -0800
@@ -911,32 +911,34 @@ js_InitGC(JSRuntime *rt, uint32 maxbytes
 JS_FRIEND_API(void)
 js_DumpGCStats(JSRuntime *rt, FILE *fp)
 {
     uintN i;
     size_t thingsPerArena;
     size_t totalThings, totalMaxThings, totalBytes;
     size_t sumArenas, sumTotalArenas;
     size_t sumFreeSize, sumTotalFreeSize;
+    JSGCArenaList *list;
+    JSGCArenaStats *stats;
 
     fprintf(fp, "\nGC allocation statistics:\n");
 
 #define UL(x)       ((unsigned long)(x))
 #define ULSTAT(x)   UL(rt->gcStats.x)
     totalThings = 0;
 
     totalMaxThings = 0;
     totalBytes = 0;
     sumArenas = 0;
     sumTotalArenas = 0;
     sumFreeSize = 0;
     sumTotalFreeSize = 0;
     for (i = 0; i < GC_NUM_FREELISTS; i++) {
-        JSGCArenaList *list = &rt->gcArenaList[i];
-        JSGCArenaStats *stats = &rt->gcStats.arenas[i];
+        list = &rt->gcArenaList[i];
+        stats = &rt->gcStats.arenas[i];
         if (stats->maxarenas == 0) {
             fprintf(fp, "ARENA LIST %u (thing size %lu): NEVER USED\n",
                     i, UL(GC_FREELIST_NBYTES(i)));
             continue;
         }
         thingsPerArena = THINGS_PER_ARENA(list->thingSize);
         fprintf(fp, "ARENA LIST %u (thing size %lu):\n",
                 i, UL(GC_FREELIST_NBYTES(i)));

Comment 7

[Igor Bukanov]

I checked in the patch from comment 6 to the trunk:

http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&cvsroot=%252Fcvsroot&date=explicit&mindate=1201644960&maxdate=1201645025&who=igor%25mir2.org