66381 - JS_MaybeGC should optimize garbage-collected/time-in-GC

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect, P2)

Description

[Brendan Eich [:brendan]]

which involves not running too often (to amortize constant costs in GC'ing) and
measuring garbage creation rate, filtering it to smooth the GC schedule, and
matching garbage collection rate.

/be

Comment 1

[Brendan Eich [:brendan]]

Will try for mozilla0.8.

/be

Comment 2

[Brendan Eich [:brendan]]

Moving out, but I will get to this in 0.9 timeframe.

/be

Comment 3

[Brendan Eich [:brendan]]

Shaver, you wanna take this one?

/be

Comment 4

[Brendan Eich [:brendan]]

shaver wants this, I know it!

/be

Comment 5

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Incremental GC, which I think we agree is the right solution to this problem,
isn't going to happen for mozilla0.9.1.  I'll try to get it working during my
period of unemployment, and post progress here.

Expect an onslaught of dependent bugs covering interim changes to the GC flags
and object locking mechanisms.

Comment 6

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Asa just told me that we shipped 0.9.2!  Who knew?

Comment 7

[chris hofmann]

->0.9.5 since we have run out of time

Comment 8

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

I don't really know when I will get this done, so I'm removing the lying
milestone marker.

Comment 9

[slice1900]

cc:ing myself

From links to it in other bugs (such as 104127) it sounds like some smart people
think this could be important to fix for performance.  Hopefully it'll at least
make moz 1.0?

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

Any GC work going to happen for 0.9.9 or 1.0?

Comment 11

[Brendan Eich [:brendan]]

shaver's on a road trip, hacking incremental gc!

/be

Comment 12

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Attachment: WIP: lock bit removal, unified sweep-and-free, mark stack and (mostly) iterative marking, reviving GC_POKE (somewhat controversial) with API addition, documentary comments for GC API points

This is not a finished work, but it lets me run all my new GC test cases and
the last version of this was happy enough in the browser.  I'm going to spin a
new build today with this latest stuff, and some other cleanups, and see how it
holds up.

People who have seen the GC show up in profiles unpleasantly might want to take
their lives in their hands and try a profile or two with this patch.  The real
money (incremental GC) is still a little ways off, but I think the iterative
marking might be a useful win anyway.  Post here with results!

Comment 13

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Attachment: All the above, plus a factored GC and the basics of incremental support.

You probably don't want to use this in software that controls medical
equipment, or even your television, but it _should_ actually work in the
browser and other apps that don't need GC_MARK_DEBUG.  Oh, and I haven't tested
it with JS_THREADSAFE at all (might not even compile!).

But I'll be damned if I'm not going to back this up everywhere I can, and
bugzilla's handy.

Comment 14

[Phil Schwartau]

On WinNT, trying to test the patch above (id=67618).
Here's what's happening:

METHOD 1
1. Pull from the tip
2. Apply the patch
3. Compile
4. Run the testsuite
5. RESULT: approx 25 regressions due to the patch; not good.


METHOD 2
1. For each file not in the patch, pull from the tip
2. For each file in the patch, pull the version mentioned there
   (for example, do cvs up -r3.42 jslock.c)
3. Apply the patch
4. Compile
5. RESULT: can't compile:

cl -FoWINNT4.0_DBG.OBJ/ -c /MD /Od /Z7  -DXP_PC -DWIN32 -D_WINDOWS -D_WIN32 
/nologo /W3 /FpWINNT4.0_DBG.OBJ/js.pch  -DDEBUG -DDEBUG_ -DEXPORT_JS_API jsstr.c  
 
jsstr.c
jsstr.c(2677) : error C2143: syntax error : missing ')' before '('
jsstr.c(2677) : error C2091: function returns function
jsstr.c(2677) : error C2373: 'memcpy' : redefinition; different type modifiers
        C:\PROGRA~1\MICROS~2\VC98\INCLUDE\string.h(101) : see declaration
                                                          of 'memcpy'
jsstr.c(2677) : error C2059: syntax error : 'type'
make[1]: *** [WINNT4.0_DBG.OBJ/jsstr.obj] Error 2
make: *** [all] Error 2


I tried hard to verify that the actual differences I have in my js/src
correspond exactly to the diffs in the patch, and they seem to. 

I don't know whether either Method 1 or Method 2 is viable.
The problem is, either way I'm not sure my JS src matches Mike's.

Mike: would it be possible to provide another patch against the current tip?
If that wouldn't be too much trouble, then I could try again...

Comment 15

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Well, that sucks.  I'm not surprised it's not clean against the trunk, but I am
surprised that I'm botching the tests now.

I'll take a copy of the test suite to the cottage this weekend.  I had some
version of this (and I thought it was the attached version) running in the
browser for a while, so there's something funky going on here.

Sorry about that, all.

Comment 16

[Phil Schwartau]

Here's the command line I use to run the tests:

[//d/JS_TRUNK/mozilla/js/tests] perl jsDriver.pl -e smdebug smopt -k -L lc2 lc3

Comment 17

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

So, with that test setup, I get 7 failures without my patch, and 6 failures
with.   (Freaky ones: Math.random, String.prototype.replace, etc.)

I'll attach another patch before I hit the cottage, updated to the tip.

Comment 18

[Phil Schwartau]

Attachment: For convenience, the "known" JS test failures on Linux

Comment 19

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Attachment: Make GC_MARK_DEBUG work, and use grey-colouring to prevent duplicate entries on the mark stack, etc.

I can pass the test suite now, kinda, and I'm spinning a Mozilla build that I
can do more testing with as I type this.

I get only 7 test failures for smdebug, and they're not all the same as phil's:

ecma_3/Function/regress-104584.js
ecma_3/RegExp/15.10.4.1-5-n.js
ecma_3/RegExp/regress-123437.js
ecma_3/String/regress-83293.js
js1_5/Array/regress-101964.js
js1_5/Exceptions/regress-123002.js
js1_5/Object/regress-90596-003.js

Of course, I passed the test suite earlier with a jsshell that would crash
horribly the second it got into GC, so I'm waiting for my Mozilla build to
complete before I get excited.

Comment 20

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Attachment: Remove bogus assertion from non-GC_MARK_DEBUG GC_PUSH.

Don't touch that last patch (#69316).  It'll stop you cold, on startup, because
of a left-over assertion in the tree I diffed to make the patch.

With the patch I'm attaching now, the browser is happy, and so am I.  Now I
just(!) need to finish the write barrier, and we're in business for 0.9.9.

Who can run Ts/Tp/Txul numbers on this for me?	I think we should see some
light speedup, but I'm mostly interested in validating my assertion that we
don't get _hurt_ by this.

Comment 21

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

I'm seeing a slight Ts win and a 1.5% Txul loss from that latest patch (even
once I get rid of the last js_LockGCThing calls in the startup path), but jrgm
is reporting huge memory consumption and other incredibly bad things when he
tries to generate Tp numbers.

More, uh, later today.  Happy Valentine's Day!

Comment 22

[Phil Schwartau]

OS type: Linux attica 2.2.12-20smp #1 SMP Mon Sep 27 10:34:45 EDT 1999 i686
gcc version 2.95.3 20010315 (release)

Mike's latest patch (69337) passes JS testsuite for me in optimized JS shell.
The only failures are the current known bugs:

Retest List, smopt
# 1020 of 1020 test(s) were completed, 9 failures reported.

ecma_3/ExecutionContexts/10.1.3-2.js  <-- new test of duplicate function params
ecma_3/RegExp/regress-85721.js        <-- new RegExp performance test
ecma_3/RegExp/regress-103087.js
ecma_3/RegExp/regress-119909.js
ecma_3/RegExp/regress-123437.js
js1_5/Object/regress-90596-001.js
js1_5/Object/regress-90596-002.js
js1_5/Object/regress-90596-003.js
js1_5/Array/regress-101964.js


HOWEVER, in the debug JS shell, I am getting many crashes on testcases
that seem to deal with numeric data. 

# Retest List, smdebug
# 1020 of 1020 test(s) were completed, 31 failures reported.
ecma/Expressions/11.10-1.js
ecma/Expressions/11.10-2.js
ecma/Expressions/11.10-3.js
ecma/Expressions/11.4.8.js
ecma/Expressions/11.7.1.js
ecma/Expressions/11.7.2.js
ecma/Expressions/11.7.3.js
ecma/GlobalObject/15.1.2.5-3.js
ecma/String/15.5.4.4-1.js
ecma/String/15.5.4.7-1.js
ecma/String/15.5.4.7-2.js
ecma/String/15.5.4.8-2.js
ecma_2/String/split-002.js
ecma_2/String/split-003.js
ecma_3/ExecutionContexts/10.1.3-2.js
ecma_3/RegExp/regress-103087.js
ecma_3/RegExp/regress-119909.js
ecma_3/RegExp/regress-123437.js
js1_2/String/slice.js
js1_2/regexp/alphanumeric.js
js1_2/regexp/digit.js
js1_2/regexp/whitespace.js
js1_2/regexp/word_boundary.js
js1_4/Regress/function-002.js
js1_5/Regress/regress-89443.js
js1_5/Regress/regress-111557.js
js1_5/Object/regress-90596-001.js
js1_5/Object/regress-90596-002.js
js1_5/Object/regress-90596-003.js
js1_5/Array/regress-101964.js
js1_5/Array/regress-107138.js

Comment 23

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

The risk/reward for 1.0 doesn't feel right, shamed though I am to still have not
solved this.  I'll attach patches as I background-hack on this, but 1.1 is the
earliest I'm looking to put this on the trunk.

I'll cut a branch for this, for easy testing by the brave and willing.

Comment 24

[ash]

I'm off to resurrect the non-recursive marking part of this patch

Comment 25

[Brendan Eich [:brendan]]

Please, bug 76831 is not a meta-bug, is way too broad a bug, and is not
obviously blocked by this bug.  Even if this bug is fixed, a swapped-out mozilla
process is likely to take a long time to page back in.

/be

Comment 26

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Mass abdication.

Comment 27

[Bob Clary [:bc] (inactive)]

Old bugs never die, they go to live in bugzilla. WONTFIXing this unless someone objects.