584704 - Built-in JSON functionality (AS3 implementation)

Status: RESOLVED WONTFIX

(Tamarin Graveyard :: Library, defect)

Description

[Lars T Hansen]

JSON encoding/decoding keeps being fingered as a bottleneck.  We could support the ES5 JSON spec natively with fairly modest effort (1-2 days).

See https://bugs.adobe.com/jira/browse/FP-1631 for the request (120 votes).

An interesting exercise in itself would be to discover why a native encoder/decoder would be faster than one written in good AS3, and under what circumstances that might or might not be the case.

Comment 1

[Lars T Hansen]

BTW simple API versioning is sufficient for this one.

Comment 2

[Lars T Hansen]

Attachment: AS3 JSON implementation

The ECMA-262-5 JSON object implemented in good AS3.

This is two things: it is an implementation, more or less ready for Tamarin, but it is also a case study in optimization - the source is heavily annotated with comments about optimizations the JIT or ASC or VM "ought to do" in order for the code to be good.  The annotations are probably incomplete, but it's a start.  The next step is to compare performance to something based on the eval code, say, and to start digging into the required optimizations - generating further PACMAN work, likely.

Comment 3

[Lars T Hansen]

Attachment: Acceptance test cases (preliminary)

Comment 4

[Lars T Hansen]

Marking a dependency on fixing for-in iteration as using that is unavoidable, yet the bugs in the iterator mechanism probably makes the JSON code unsafe / unsound.

Comment 5

[Lars T Hansen]

Bug #588922 tracks many optimizations needed for this implementation.

Comment 6

[Lars T Hansen]

Regarding some of the optimization notes in the code:

- tamarin avoids allocation for "" + s and s + ""
- tamarin avoids allocation if substring() extracts the whole string

Comment 7

[Lars T Hansen]

Regarding some of the optimization notes in the code:

- tamarin optimizes AS3::call so that it performs no heap allocation.
  (Function.prototype.call is not so lucky but does not matter for 
  the JSON code.)

Comment 8

[Lars T Hansen]

I propose that if we do a C++ JSON implementation a new bug is created for that.

Comment 9

[Felix S. Klock II [:pnkfelix, :fklock]]

(In reply to comment #8)
> I propose that if we do a C++ JSON implementation a new bug is created for
> that.

Posted as Bug 640711.

Comment 10

[Steven Johnson]

Comment on attachment 464839 [details]
AS3 JSON implementation

"It is possible to do "new JSON" because we're not allowed by AS3 to hide the constructor" -- no longer true, we can now use [native(construct="none")] to prevent this.

Comment 11

[Lars T Hansen]

(In reply to comment #10)
> Comment on attachment 464839 [details]
> AS3 JSON implementation
> 
> "It is possible to do "new JSON" because we're not allowed by AS3 to hide the
> constructor" -- no longer true, we can now use [native(construct="none")] to
> prevent this.

That misses the point - that's outside the language.  The purpose here was to be an entirely in-AS3 implementation.

Comment 12

[Felix S. Klock II [:pnkfelix, :fklock]]

(In reply to comment #11)
> (In reply to comment #10)
> > Comment on attachment 464839 [details]
> > AS3 JSON implementation
> > 
> > "It is possible to do "new JSON" because we're not allowed by AS3 to hide the
> > constructor" -- no longer true, we can now use [native(construct="none")] to
> > prevent this.
> 
> That misses the point - that's outside the language.  The purpose here was to
> be an entirely in-AS3 implementation.

But on a side-note, *sweet*: [native(construct="none")] is exactly the approach I took when I ported Lars's code to a builtin; see patch B in Bug 640711.  :)

Comment 13

[Felix S. Klock II [:pnkfelix, :fklock]]

FYI this JSON parser will infinite-loop for JSON.parse("[").  (Thanks to dschaffe for this discovery.)  parseArray and parseValue mutually recur ad infinitum.

The reason is that the token is set to '[' (aka 93) on the initial advance() invocation, and then every subsequent advance() invocation makes no progress forward but also leaves the token set at '['. 

I think one reasonable fix is to reset the token at the start of advance() before entering the loop.  There may be others that are less obscure.

Comment 14

[Felix S. Klock II [:pnkfelix, :fklock]]

Attachment: fix some bugs in lars impl

12345678901234567890123456789012345678901234567890123456789012345678901234567890

Bugs fixed:
1. Avoid infinite regress mentioned in comment 13 by resetting token to 0 at 
   start of advance()
2. Fix unicode escape parsing, which was previously guaranteed to throw syntax
   errors.
3. Throw syntax error for JSON.parse("1 2") -- I am hoping that the change
   also covers the more general case of any input with multiple objects 
   separated by whitespace, but I am not 100% convinced of it yet. 

(throwing a patch up here was most convenient way for me to stash these where 
I can be sure to snag them later when I propagate the fixes to the C++ code
for Bug 640711.)

Comment 15

[Tamarin Bot]

changeset: 6276:b4a71849a889
user:      Felix S Klock II <[e-mail]>
summary:   Bug 654574: JSON left out replacer Array prepass (original=lhansen, adapted by fklockii, r=fklockii).

http://hg.mozilla.org/tamarin-redux/rev/b4a71849a889

Comment 16

[Sylvestre Ledru [:Sylvestre]]

Tamarin is a dead project now. Mass WONTFIX.

Comment 17

[Sylvestre Ledru [:Sylvestre]]

Tamarin isn't maintained anymore. WONTFIX remaining bugs.