std.data.json formal review

[Sönke Ludwig via Digitalmars-d]

Am 12.08.2015 um 08:28 schrieb Dmitry Olshansky:
> On 12-Aug-2015 00:21, Sönke Ludwig wrote:
>> Am 11.08.2015 um 20:15 schrieb Dmitry Olshansky:
>>> On 11-Aug-2015 20:30, deadalnix wrote:
>>>>
>>>> Ok some actionable items.
>>>>
>>>> 1/ How big is a JSON struct ? What is the biggest element in the
>>>> union ?
>>>> Is that element really needed ? Recurse.
>>>
>>> +1 Also most JS engines use nan-boxing to fit type tag along with the
>>> payload in 8 bytes total. At least the _fast_ path of std.data.json
>>> should take advantage of similar techniques.
>>
>> But the array field already needs 16 bytes on 64-bit systems anyway. We
>> could surely abuse some bits there to at least not use up more for the
>> type tag, but before we go that far, we should first tackle some other
>> questions, such as the allocation strategy of JSONValues during parsing,
>> the Location field and BigInt/Decimal support.
>
> Pointer to array should work for all fields > 8 bytes. Depending on the
> ratio frequency of value vs frequency of array (which is at least an
> ~5-10 in any practical scenario) it would make things both more compact
> and faster.
>
>> Maybe we should first have a vote about whether BigInt/Decimal should be
>> supported or not, because that would at least solve some of the
>> controversial tradeoffs. I didn't have a use for those personally, but
>> at least we had the real-world issue in vibe.d's implementation that a
>> ulong wasn't exactly representable.
>
> Well I've stated why I think BigInt should be optional. The reason is
> C++ parsers don't even bother with anything beyond ULong/double, nor
> would any e.g. Node.js stuff bother with things beyond double.

The trouble begins with long vs. ulong, even if we'd leave larger 
numbers aside. We'd really have to support both, but choosing between 
the two is ambiguous, which isn't very pretty overall.

>
> Lastly we don't have BigFloat so supporting BigInt but not BigFloat is
> kinda half-way.

That's where Decimal would come in. There is some code for that 
commented out, but I really didn't want to add it without a standard 
Phobos implementation. But I wouldn't say that this is really an 
argument against BigInt, maybe more one for implementing a Decimal type.

>
> So please make it an option. And again add an extra indirection (that is
> BigInt*) for BigInt field in a union because they are extremely rare.

Good idea, didn't think about that.

>
>> My view generally still is that the DOM representation is something for
>> convenient manipulation of small chunks of JSON, so that performance is
>> not a priority, but feature completeness is.
>
> I'm confused - there must be some struct that represents a useful value.

There is also the lower level JSONParserNode that represents data of a 
single bit of the JSON document. But since that struct is just part of a 
range, its size doesn't matter for speed or memory consumption (they are 
not allocated or copied while parsing).

> And more importantly - is JSONValue going to be converted to jsvar? If
> not - I'm fine. Otherwise whatever inefficiency present in JSONValue
> would be accumulated by this conversion process.

By default and currently it isn't, but it might be an idea for the 
future. The jsvar struct could possibly be implemented as a wrapper 
around JSONValue as a whole, so that it doesn't have to perform an 
actual conversion of the whole document.

Generally, working with JSONValue is already rather inefficient due to 
all of the dynamic allocations to populate dynamic and associative 
arrays. Changing that would require switching to completely different 
underlying container types, which would at least make the API a lot less 
intuitive.

We could of course also simply provide an alternative value 
representation that is not based on Algebraic (or an enum tag based 
alternative) and is not augmented with location information, but 
optimized solely for speed and low memory consumption.