fleece-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 11:56:49 GMT
-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).

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).

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.

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

Mime
View raw message