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: PULL 20
Date Tue, 03 Apr 2018 15:55:47 GMT
yes, the two blocks at
https://github.com/apache/johnzon/pull/20/files#diff-7f654235e6be2d22d4eb2df20cd40b19R153
and
https://github.com/apache/johnzon/pull/20/files#diff-7f654235e6be2d22d4eb2df20cd40b19R297

concretely we want to bypass the bounds access once we already got them
once because it goes down to byte[] parsing and it slows down artificially
the runtime.
Same for class.isEnum() which leads to a native (and we can cache that
aggressively in the ClassMapping) - that said there is not only the one you
added here when rechecking ;).

If you think it is ok to keep it as it - can be since my comments are from
past experience but was on pre java 8 tests, you can add a bench as a unit
test in a small project which shows we dont go slower (we can accept a
range of 5% of error) for the old and new code paths. It is always the best
way to solve the perf questions ;).


Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<https://www.packtpub.com/application-development/java-ee-8-high-performance>

2018-04-03 17:43 GMT+02:00 Alessandro Moscatelli <
alessandro.moscatelli@live.com>:

> By “find” do you mean “findObjectConverterReader” ?
>
>
>
> If so, they are called only in 2 very specific cases, and in these cases
> en exception was throwed by :
>
>
>
>         if (classMapping == null) {
>
>             throw new MapperException("Can't map " + type);
>
>         }
>
>
>
> To be accurate I am speaking about :
>
> CASE1) classMapping == null AND ParameterizedType.class.isInstance(type)
> AND fieldArgTypes.length <= 1
> CASE2) classMapping == null AND !ParameterizedType.class.isInstance(type)
> AND TypeVariable.class.isInstance(type)
>
>
>
> These 2 cases ALWAYS ended with errors before my commit.
>
>
> Are you sure you want have a switch for these ?
> Anyway the switch is fine for me.
>
>
>
> Thank you
> AM
>
>
>
> ________________________________
> Da: Romain Manni-Bucau <rmannibucau@gmail.com>
> Inviato: Tuesday, April 3, 2018 5:03:16 PM
> A: dev@johnzon.apache.org
> Oggetto: Re: PULL 20
>
> Hi Alessandro,
>
> Is it possible to reduce all the finds you do at runtime? If not we can
> need a flag to switch if off by default since it will slow down the
> processing in most cases for no gain (for these cases).
> Overall idea is to do all the reflection we can once only since types can
> be cached per mapper/jsonb instance.
>
> Can you check it out to avoid a perf regression please?
>
> otherwise, other parts looks pretty ready to merge.
>
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> <https://rmannibucau.metawerx.net/> | Old Blog
> <http://rmannibucau.wordpress.com> | Github <https://github.com/
> rmannibucau> |
> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
> <https://www.packtpub.com/application-development/java-
> ee-8-high-performance>
>
> 2018-04-03 16:45 GMT+02:00 Alessandro Moscatelli <
> alessandro.moscatelli@live.com>:
>
> > Hi there,
> > this is my third and final (for now) pull.
> > I made some minor fixes to enable borderline scenarios like
> > deserialization of enum from deserializationContext.
> > This jsonb interface can only be access inside jsonb deserializers, so
> > these scenarios maybe are not so common or so much tested.
> > I also made a Test specific for this.
> >
> > I also pushed a small fix for milli/micro/nano seconds being non
> > considerated/truncated and I made a test for this too.
> >
> > Please let me know as soon as you can if there are any problems.
> > Thank you
> > AM
> >
> > Da: Alessandro Moscatelli<mailto:alessandro.moscatelli@live.com>
> > Inviato: venerdì 30 marzo 2018 16:30
> > A: dev@johnzon.apache.org<mailto:dev@johnzon.apache.org>
> > Oggetto: PULL 19 - complete support for DateFormat
> >
> >
> > Hi there,
> > here is my second pull.
> > It’s not a refactor, I simply extended the already present implementation
> > to provide support and apply formatting for more Date like types
> > (serialization and deserialization) :
> >
> > Date
> > LocalDateTime
> > LocalDate
> > OffsetDateTime
> > ZonedDateTime
> > Calendar
> > GregorianCalendar
> > Instant
> >
> > I also noticed that DateFormat were not used for serialized, in any case
> > (default value was Always used, and this was a blocking bug for me).
> >
> > After this I have a final pull.
> > Please let me know as soon as you can if there are any problems.
> > Greetings
> > AM
> >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message