johnzon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Romain Manni-Bucau <rmannibu...@gmail.com>
Subject Re: JsonValue immutability
Date Tue, 15 Jul 2014 13:07:28 GMT
2014-07-15 14:37 GMT+02:00 Hendrik Dev <hendrikdev22@gmail.com>:
> i agree in general but:
>
>
> On Tue, Jul 15, 2014 at 1:56 PM, Romain Manni-Bucau
> <rmannibucau@gmail.com> wrote:
>> -0
>>
>> personally I find this Json* API relatively wrong (starting from the
>> fact it inherits from Map or List so we *need* to extend an
>> implementation to not break upgrading jvm version , it also means we
>> can be immutable with java 6-7 but we'll not since you work with java
>> 8 since and we can't protect against its new methods).
>

When I did it I thought: let it be spec compliant with java 7 and run
fine on java 8 even if immutability is a bit broken. Think it is
acceptable today (at least for first releases)

> That a point in fact if have until now not fully recognized. You're
> right, thats very bad API design.
> I just tend to say that we may want to consider splitting up the
> implementations in a codebase per java major version :-(
> (Thats the point i want have good old conditional compiling :-)
>
>>
>> I'd 100% rely on TCKs when we'll get them to do as few as necessary to
>> be spec compliant on this topic (we can also check the RI on this
>> point).
>
> when do we get the TCKs?

Apache is discussing with Oracle ATM.

> Have not looked into RI yet (to be uninfluenced) but think thats a
> good idea here.
>
>>
>> If you see we can improve performances doing it we should go for it,
>> if that's just to be nice not sure it worths it (it will create more
>> objects with very few gain and it is IMHO a drawback).
>>
>> About thread safety: I think we don't have to be thread safe at this level.
>
> I think we need it here because different threads can see different
> hashcodes otherwise
> (http://invalidcodeexception.com/do-immutability-really-means-thread-safety/)
> and thats really an issue especially if JsonValues are stored in
> collections.
>

Shouldn't since once you are added in a collection you are immutable.

> Regards
> Hendrik
>
>>
>> does it make sense in your opinion?
>>
>>
>>
>> Romain Manni-Bucau
>> Twitter: @rmannibucau
>> Blog: http://rmannibucau.wordpress.com/
>> LinkedIn: http://fr.linkedin.com/in/rmannibucau
>> Github: https://github.com/rmannibucau
>>
>>
>> 2014-07-15 13:39 GMT+02:00 Hendrik Dev <hendrikdev22@gmail.com>:
>>> To open another discussion ;-) i looked on the JsonValue
>>> implementations regarding immutability (as the API requires).
>>>
>>> Looks like that doing real immutability have a really bad performance
>>> impact (at least in my prototype implementation which is here
>>> https://github.com/salyh/fleece_master/commit/8ce227915ef5bcaa695fa354b7126762a2221d35).
>>> So i understand why the initial implementation deals with the
>>> JsonArrayImpl.addInternal() and JsonObjectImpl.putInternal(). But if i
>>> were a nitpicker i would say thats not immutable :-)
>>> The main performance impact is due to the builder implementations i
>>> think if the values are really immutable.
>>>
>>> But at least the JsonValue implementations itself can be immutable
>>> without big perf drawbacks hopefully. To make them "more" immutable
>>> (and keep the put/addInternal()) i suggest:
>>>
>>> For JsonArrayImpl
>>> - getValuesAs() should return unmodifiable list
>>> https://github.com/salyh/fleece_master/commit/8ce227915ef5bcaa695fa354b7126762a2221d35#commitcomment-7011110
>>>
>>> - subList(int fromIndex, int toIndex) should return unmodifiable list
>>> https://github.com/salyh/fleece_master/commit/8ce227915ef5bcaa695fa354b7126762a2221d35#commitcomment-7011145
>>>
>>> - listIterator(int index) disallow to change list
>>> https://github.com/salyh/fleece_master/commit/8ce227915ef5bcaa695fa354b7126762a2221d35#commitcomment-7011156
>>>
>>> - make class final
>>>
>>> - make hashcode caching threadsafe (int terms of "volatile")
>>> https://github.com/salyh/fleece_master/commit/8ce227915ef5bcaa695fa354b7126762a2221d35#commitcomment-7011019
>>>
>>> - make addInternal package private
>>>
>>>
>>> For JasonObjectImpl
>>> - keySet() should return unmodifiable set
>>>
>>> - values()  should return unmodifiable collection
>>>
>>> - entrySet() should return unmodifiable set (tricky ->
>>> http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/7-b147/java/util/Collections.java#1366)
>>>
>>> - make class final
>>>
>>> - make hashcode caching threadsafe (int terms of "volatile")
>>> https://github.com/salyh/fleece_master/commit/8ce227915ef5bcaa695fa354b7126762a2221d35#commitcomment-7011019
>>>
>>> - make putInternal package private
>>>
>>>
>>> For the other JsonValue implementations
>>> - make classes final
>>> - make hashcode caching threadsafe (int terms of "volatile")
>>> https://github.com/salyh/fleece_master/commit/8ce227915ef5bcaa695fa354b7126762a2221d35#commitcomment-7011019
>>>
>>> Your opinion?
>>>
>>> Regards
>>> Hendrik
>>>
>>>
>>>
>>> --
>>> Hendrik Saly (salyh, hendrikdev22)
>>> @hendrikdev22
>>> PGP: 0x22D7F6EC
>
>
>
> --
> Hendrik Saly (salyh, hendrikdev22)
> @hendrikdev22
> PGP: 0x22D7F6EC

Mime
View raw message