2014-07-15 14:37 GMT+02:00 Hendrik Dev <[email protected]>: > i agree in general but: > > > On Tue, Jul 15, 2014 at 1:56 PM, Romain Manni-Bucau > <[email protected]> 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 <[email protected]>: >>> 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
