Bug 155351 – [ES6] Add support for Symbol.isConcatSpreadable.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Keith Miller]

We should support this.

[Keith Miller]

Created attachment 274305 [details]
Patch

[Keith Miller]

Created attachment 274308 [details]
Patch

[WebKit Commit Bot]

Attachment 274308 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGNodeType.h:295:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 43 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Keith Miller]

Created attachment 274310 [details]
Benchmark results

[Keith Miller]

Results with 10 reruns on Octane/Kraken (I assume json-parse-financial is unrelated):

                                                   TOT                      Conf#2
Octane:
   encrypt                                   1.48749+-0.04154          1.47919+-0.06296
   decrypt                                  26.80845+-0.61037         26.76883+-0.73212
   deltablue                        x2       1.28161+-0.02215    ?     1.28218+-0.05821       ?
   earley                                    2.69246+-0.01478          2.68168+-0.02034
   boyer                                    44.10906+-2.09822         43.41107+-0.24304         might be 1.0161x faster
   navier-stokes                    x2      45.87185+-0.69001    ?    46.03705+-0.98245       ?
   raytrace                         x2       8.37932+-0.07353          8.35955+-0.03899
   richards                         x2       0.76783+-0.02436          0.76344+-0.01918
   splay                            x2       3.26142+-0.05023          3.25891+-0.07848
   regexp                           x2     190.28251+-2.03038    ?   190.34545+-14.83350      ?
   pdfjs                            x2     346.79246+-8.67128    ?   349.91664+-5.86553       ?
   mandreel                         x2     362.50135+-3.07694        361.60019+-2.45072
   gbemu                            x2     223.84159+-0.79068    ?   224.27118+-1.42336       ?
   closure                                   5.46698+-0.02357    ?     5.47181+-0.03893       ?
   jquery                                   70.71302+-1.24316    ?    71.11153+-0.94862       ?
   box2d                            x2      86.37910+-0.75527         86.27665+-0.85182
   zlib                             x2    2035.71659+-23.94612      2021.69727+-34.57433
   typescript                       x2    3794.22253+-115.38443     3786.35999+-27.57660

   <geometric>                              44.84098+-0.22196         44.78911+-0.41718         might be 1.0012x faster

                                                   TOT                      Conf#2
Kraken:
   ai-astar                                  838.869+-26.346     ?     839.639+-34.228        ?
   audio-beat-detection                      448.569+-2.983      ?     452.673+-1.979         ?
   audio-dft                                 836.956+-7.875      ?     842.762+-10.828        ?
   audio-fft                                 352.547+-1.394      ?     358.491+-6.047         ? might be 1.0169x slower
   audio-oscillator                          446.141+-2.810            445.885+-2.802
   imaging-darkroom                          572.133+-13.260           567.848+-8.067
   imaging-desaturate                        424.027+-13.189     ?     430.323+-37.076        ? might be 1.0148x slower
   imaging-gaussian-blur                     591.073+-15.968           587.380+-32.795
   json-parse-financial                      344.676+-4.265      !     353.683+-4.299         ! definitely 1.0261x slower
   json-stringify-tinderbox                  212.391+-3.579      ?     217.195+-8.337         ? might be 1.0226x slower
   stanford-crypto-aes                       386.629+-7.136            384.526+-5.071
   stanford-crypto-ccm                       323.469+-9.658            302.488+-11.661          might be 1.0694x faster
   stanford-crypto-pbkdf2                    922.743+-7.144            911.586+-14.156          might be 1.0122x faster
   stanford-crypto-sha256-iterative          370.241+-4.056      ^     357.123+-5.823         ^ definitely 1.0367x faster

   <arithmetic>                              505.033+-2.092            503.686+-2.581           might be 1.0027x faster

                                                   TOT                      Conf#2
Geomean of preferred means:
   <scaled-result>                          150.4864+-0.6535          150.1984+-0.9704          might be 1.0019x faster

[Filip Pizlo]

Comment on attachment 274308 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=274308&action=review

> Source/JavaScriptCore/ChangeLog:16
> +        4) ToObject: Attempts to convert the first child into an object.

It would help the summary if you said here what happens when ToObject fails in its attempt: returns null?  OSR exits?

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1054
> +            if (!(child.m_type & ~SpecArray)) {
> +                setConstant(node, jsBoolean(true));
> +                constantWasSet = true;
> +                break;
> +            }
> +

Nice.  You could also return false if !(m_child.m_type & SpecObject).

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2124
> +        ASSERT(argumentCountIncludingThis == 2);

So, you're asserting because this is only used from builtins?

I'm not sure I like that. :-(  I think that it's nice to have reduce the strength of the contract between DFG and builtins, so that if you call a builtin with more args, you don't crash.

> Source/JavaScriptCore/dfg/DFGClobberize.h:426
> -        
> +

Revert

[Build Bot]

Comment on attachment 274308 [details]
Patch

Attachment 274308 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/995461

New failing tests:
js/dom/array-prototype-properties.html

[Build Bot]

Created attachment 274315 [details]
Archive of layout-test-results from ews100 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 274308 [details]
Patch

Attachment 274308 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/995467

New failing tests:
js/dom/array-prototype-properties.html

[Build Bot]

Created attachment 274317 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 274308 [details]
Patch

Attachment 274308 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/995466

New failing tests:
js/dom/array-prototype-properties.html

[Build Bot]

Created attachment 274318 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 274308 [details]
Patch

Attachment 274308 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/995494

New failing tests:
inspector/codemirror/prettyprinting-javascript.html
jquery/manipulation.html
jquery/traversing.html
js/kde/Array.html
inspector/console/command-line-api.html
js/dom/array-prototype-properties.html

[Build Bot]

Created attachment 274321 [details]
Archive of layout-test-results from ews113 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Keith Miller]

Comment on attachment 274308 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=274308&action=review

>> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2124
>> +        ASSERT(argumentCountIncludingThis == 2);
> 
> So, you're asserting because this is only used from builtins?
> 
> I'm not sure I like that. :-(  I think that it's nice to have reduce the strength of the contract between DFG and builtins, so that if you call a builtin with more args, you don't crash.

We also have the same assertion in the native function implementation as well. I would agree that such an assertion is not ideal were that the not the case. As it stands right now it's just a continuation of the same sanity check.

>> Source/JavaScriptCore/dfg/DFGClobberize.h:426
>> +
> 
> Revert

Fixed.

[Keith Miller]

Comment on attachment 274308 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=274308&action=review

>> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1054
>> +
> 
> Nice.  You could also return false if !(m_child.m_type & SpecObject).

Good call. I've added that case.

[Keith Miller]

Created attachment 274343 [details]
Patch

[WebKit Commit Bot]

Attachment 274343 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGNodeType.h:295:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Keith Miller]

Created attachment 274348 [details]
Patch

[WebKit Commit Bot]

Attachment 274348 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGNodeType.h:295:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Saam Barati]

Comment on attachment 274343 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=274343&action=review

> Source/JavaScriptCore/builtins/ArrayPrototype.js:655
> +    var resultIsArray = @isActualArray(result);

Maybe "isUnproxiedArray"? Or "isPlainArray"?

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:974
> +                        setConstant(node, jsBoolean(true));

Do you want to fall through here?

> Source/JavaScriptCore/dfg/DFGNodeType.h:295
> +    /* I'd like to call this IsArray but then we get namespace problems with the indexing type name. Also, it is marked must generate because it can throw. */ \

nit: not sure the namespace conflicts part of the comment is helpful.

[Keith Miller]

Created attachment 274351 [details]
Patch

[WebKit Commit Bot]

Attachment 274351 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGNodeType.h:295:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 274351 [details]
Patch

Attachment 274351 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/996764

New failing tests:
js/dom/array-prototype-properties.html

[Build Bot]

Created attachment 274355 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 274351 [details]
Patch

Attachment 274351 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/996769

New failing tests:
js/dom/array-prototype-properties.html

[Build Bot]

Created attachment 274356 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 274351 [details]
Patch

Attachment 274351 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/996813

New failing tests:
js/dom/array-prototype-properties.html

[Build Bot]

Created attachment 274359 [details]
Archive of layout-test-results from ews100 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 274351 [details]
Patch

Attachment 274351 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/996773

New failing tests:
js/dom/array-prototype-properties.html

[Build Bot]

Created attachment 274360 [details]
Archive of layout-test-results from ews114 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Keith Miller]

Tests just need to be rebaselined I'll do that before landing.

[Saam Barati]

Comment on attachment 274351 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=274351&action=review

I have some comments. I haven't looked over everything, but I think there is enough here to address.

> Source/JavaScriptCore/builtins/ArrayPrototype.js:655
> +    var resultIsArray = @isActualArray(result);

I still like "isUnproxiedArray" for the name of this function.

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1836
> +        destination.merge(SpecObject);

Why merge here and not set the type to be SpecObject?

> Source/JavaScriptCore/dfg/DFGNodeType.h:295
> +    /* I'd like to call this IsArray but then we get namespace problems with the indexing type name. Also, it is marked must generate because it can throw. */ \

Still not a fan of the namespace comment.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3302
> +    JSValueOperand value(this, node->child1());
> +    GPRFlushedCallResult result(this);
> +
> +    JSValueRegs valueRegs = value.jsValueRegs();
> +    GPRReg resultGPR = result.gpr();
> +
> +    JITCompiler::Jump isNotCell = m_jit.branchIfNotCell(valueRegs);
> +
> +    JITCompiler::Jump notArray = m_jit.branch8(JITCompiler::NotEqual,
> +        JITCompiler::Address(valueRegs.payloadGPR(), JSCell::typeInfoTypeOffset()),
> +        TrustedImm32(ArrayType));
> +    moveTrueTo(resultGPR);
> +    JITCompiler::Jump isArrayObject = m_jit.jump();
> +
> +    notArray.link(&m_jit);
> +    flushRegisters();
> +    callOperation(operationIsArrayObject, resultGPR, valueRegs);
> +    blessBoolean(resultGPR);
> +    JITCompiler::Jump isProxyObject = m_jit.jump();
> +
> +    isNotCell.link(&m_jit);
> +    moveFalseTo(resultGPR);
> +
> +    isProxyObject.link(&m_jit);
> +    isArrayObject.link(&m_jit);
> +    blessedBooleanResult(resultGPR, node);

I'm not sure your use of flushRegisters is correct here. I think it should be a silentFlush because
there are paths that don't need flushing. Also, the names of your jumps are confusing to me.
I think it's be easier to read just if you had a done JumpList. It might also be worth adding a FIXME
to inline all of "isArray" here and only jump to the slow path when we've encountered a revoked proxy
so we can throw an exception.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3316
> +void SpeculativeJIT::compileIsArrayConstructor(Node* node)
> +{
> +    JSValueOperand value(this, node->child1());
> +    GPRFlushedCallResult result(this);
> +
> +    JSValueRegs valueRegs = value.jsValueRegs();
> +    GPRReg resultGPR = result.gpr();
> +
> +    flushRegisters();
> +    callOperation(operationIsArrayConstructor, resultGPR, valueRegs);
> +    unblessedBooleanResult(resultGPR, node);
> +}

I just looked at the code for "arrayConstructorPrivateFuncIsArrayConstructor" and I feel like we could do a lot better here.
That function just does a JSDyanmicCast on ArrayConstructor, but I don't think we have any inheritors of that
class. Why not just do a class info equality check here instead?

> Source/JavaScriptCore/runtime/ArrayConstructor.h:100
> +    {
> +        if (!argumentValue.isObject())
> +            return false;
> +
> +        return jsCast<JSObject*>(argumentValue)->classInfo() == ArrayConstructor::info();
> +    }

Style: indentation.

> Source/JavaScriptCore/runtime/ArrayPrototype.cpp:1029
> +        // We can

Unfinished comment.

> Source/JavaScriptCore/runtime/ArrayPrototype.cpp:1036
> +            if (vm.exception())
> +                return false;

can this throw?

> Source/JavaScriptCore/runtime/ArrayPrototype.cpp:1080
> +        auto buffer = result->butterfly()->contiguousDouble().data();

I know we use auto elsewhere for similar operations, but I think it's easier to verify the memcpy is correct if you wrote out "double* buffer" here and below.

[Keith Miller]

Comment on attachment 274351 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=274351&action=review

>> Source/JavaScriptCore/builtins/ArrayPrototype.js:655
>> +    var resultIsArray = @isActualArray(result);
> 
> I still like "isUnproxiedArray" for the name of this function.

I like IsActualArray since I plan, in the near future, to make runtime arrays not be JSArrays anymore so IsActualArray will just refer to ArrayPrototype and JSArrays. As a note, I'd like to make runtime arrays not subclasses of JSArrays since they are different enough in how they are laid out it becomes inconvenient to deal with. I would consider renaming it to isJSArray, however, what are your thoughts that name?

>> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1836
>> +        destination.merge(SpecObject);
> 
> Why merge here and not set the type to be SpecObject?

That's a good point. I was copying ToThis bock that was probably a mistake since ToThis is poorly implemented. I'll change.

>> Source/JavaScriptCore/dfg/DFGNodeType.h:295
>> +    /* I'd like to call this IsArray but then we get namespace problems with the indexing type name. Also, it is marked must generate because it can throw. */ \
> 
> Still not a fan of the namespace comment.

I think it's useful as someone might want to change the name to IsArray in the future. It would be annoying to spend the time trying to change the name only to find out it doesn't work.

>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3302
>> +    blessedBooleanResult(resultGPR, node);
> 
> I'm not sure your use of flushRegisters is correct here. I think it should be a silentFlush because
> there are paths that don't need flushing. Also, the names of your jumps are confusing to me.
> I think it's be easier to read just if you had a done JumpList. It might also be worth adding a FIXME
> to inline all of "isArray" here and only jump to the slow path when we've encountered a revoked proxy
> so we can throw an exception.

I think you are correct about the spilling. I'll change them all to silent spill/fill. Although, for now, perhaps we should just make all the non-JSArray cases slow paths. If I was going to remove runtime array from subclassing JSArray the fast path you proposed would not really work since we would still need to look for runtime arrays.

>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3316
>> +}
> 
> I just looked at the code for "arrayConstructorPrivateFuncIsArrayConstructor" and I feel like we could do a lot better here.
> That function just does a JSDyanmicCast on ArrayConstructor, but I don't think we have any inheritors of that
> class. Why not just do a class info equality check here instead?

I didn't bother making this fast because once we get to the DFG we nearly always prove that the node is a constant. We have 0 benchmarks that will call this code. I'll add a FIXME to make the change though.

>> Source/JavaScriptCore/runtime/ArrayConstructor.h:100
>> +    }
> 
> Style: indentation.

Fixed.

>> Source/JavaScriptCore/runtime/ArrayPrototype.cpp:1029
>> +        // We can
> 
> Unfinished comment.

Whoops, thanks.

>> Source/JavaScriptCore/runtime/ArrayPrototype.cpp:1036
>> +                return false;
> 
> can this throw?

Yes, if we run out of memory.

>> Source/JavaScriptCore/runtime/ArrayPrototype.cpp:1080
>> +        auto buffer = result->butterfly()->contiguousDouble().data();
> 
> I know we use auto elsewhere for similar operations, but I think it's easier to verify the memcpy is correct if you wrote out "double* buffer" here and below.

That's fair I guess. I'll make the change.

[Keith Miller]

Created attachment 274475 [details]
Patch

[WebKit Commit Bot]

Attachment 274475 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGNodeType.h:295:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 46 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Saam Barati]

Comment on attachment 274475 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=274475&action=review

r=me with comment

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3296
> +    silentFillAllRegisters(resultGPR);

You're missing an exception check here after the silentFillAllRegisters call.
This also means we probably don't have a test for this. I think it's worth
adding one. And it's probably as easy as making the proxy revoked after it
gets DFG compiled.

[Keith Miller]

Created attachment 275128 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 275128 [details]
Patch for landing

Rejecting attachment 275128 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 275128, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
hunk FAILED -- saving rejects to file LayoutTests/js/Object-getOwnPropertyNames-expected.txt.rej
patching file LayoutTests/js/dom/array-prototype-properties-expected.txt
patching file LayoutTests/js/script-tests/Object-getOwnPropertyNames.js
Hunk #1 FAILED at 70.
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/js/script-tests/Object-getOwnPropertyNames.js.rej

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.webkit.org/results/1065122

[Keith Miller]

Created attachment 275131 [details]
Patch for landing

[Keith Miller]

Created attachment 275139 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 275139 [details]
Patch for landing

Clearing flags on attachment: 275139

Committed r198808: <http://trac.webkit.org/changeset/198808>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Benjamin Poulain]

The bots show a 3% regression on JSBench.
This is likely the cause. Can you please have a look?

[David Kilzer (:ddkilzer)]

(In reply to comment #44)
> The bots show a 3% regression on JSBench.
> This is likely the cause. Can you please have a look?

And it caused 16 JSC tests to fail on 32-bit testers:

Bug 156016: REGRESSION (r198808): 16 32-bit JSC tests fail after adding support for ES6 Symbol.isConcatSpreadable
<https://bugs.webkit.org/show_bug.cgi?id=156016>

[Keith Miller]

I'm going to rollout the patch since there appear to be nontrivial issues to fix.

[Keith Miller]

Rolled out in <http://trac.webkit.org/changeset/198844>

[Csaba Osztrogonác]

Reopen, because it was rolled out.

[Keith Miller]

Committed r199128: <http://trac.webkit.org/changeset/199128>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 156348

[Keith Miller]

Committed r199397: <http://trac.webkit.org/changeset/199397>

[Alexey Proskuryakov]

This change caused many crashes and other failures, rolling out.

https://build.webkit.org/results/Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/r199397%20(4385)/results.html

[Keith Miller]

I think I have a fix running tests now.

[Alexey Proskuryakov]

Also this, I think: https://build.webkit.org/builders/Apple%20El%20Capitan%2032-bit%20JSC%20%28BuildAndTest%29/builds/2076/steps/webkit-32bit-jsc-test/logs/stdio

[Csaba Osztrogonác]

And the ARM bot too: 
https://build.webkit.org/builders/JSCOnly%20Linux%20ARMv7%20Traditional%20Release/builds/241

[Csaba Osztrogonác]

just to document, fixed by https://trac.webkit.org/changeset/199402