Change default of MapperFeature.DEFAULT_VIEW_INCLUSION to false in 3.0

[sdeleuze]

In Jackson 2.x, MapperFeature.DEFAULT_VIEW_INCLUSION is set to true by default, but almost all users set it to false because the most common use case is to only keep properties or fields annotated with @JsonView.

In Spring Framework and Spring Boot, we already modify this default value, but maybe the default setting could be modify in Jackson 3.x in order to avoid confusing users?

[cowtowncoder]

Thank you for suggesting this: I hope to focus bit more on 3.x planning after getting 2.9 finalized.
It's great to have these entries as a starting point. Next step would probably be to either create a new repo, or to use a wiki of existing repos (maybe jackson-docs or jackson or just jackson-databind); either way, having a collaborative sharing environment for collecting ideas and plans.

[cowtowncoder]

Being discussed here:

• Jackson 3.0.0 default for `MapperFeature.DEFAULT_VIEW_INCLUSION` jackson-future-ideas#75

based on which change will either made, or not.

[cowtowncoder]

Discussion closed: will proceed with this change.

[JooHyukKim]

@cowtowncoder let's add below to JSTEP-2. Leaving this here because, Currently laptop is blocked to edit anything from github.com. Will comeback and pick this up in 2 days 🥲.

* `DEFAULT_VIEW_INCLUSION`: default to `false` (because the most common use case is to only keep properties or fields annotated with `@JsonView`.)
    * [databind#1484](https://github.com/FasterXML/jackson-databind/issues/1484)

[cowtowncoder]

@JooHyukKim Will add once it gets completed -- having some problems with PR (#4885).

Basically PR initially had 12 fails; could fix/work-around 7 by configuring ObjectMapper for tests, but while all could in theory be patched, there does seem to be something odd in "not included by default" mode, changing behavior when there should be no change.
Some of failures happen in 2.19 as well but not all -- and none of these make immediate sense.

[JooHyukKim]

Right, I pulled and checked. Below tests are failing.

[ERROR] Failures: 
[ERROR]   CollectionDeserTest.testArrayIndexForExceptions:298 expected: <keys> but was: <>
[ERROR]   TestExceptionHandlingWithDefaultDeserialization.testShouldThrowExceptionWithPathReference:58 expected: <tools.jackson.databind.exc.TestExceptionHandlingWithDefaultDeserialization$Foo["bar"]->tools.jackson.databind.exc.TestExceptionHandlingWithDefaultDeserialization$Bar["baz"]> but was: <tools.jackson.databind.exc.TestExceptionHandlingWithDefaultDeserialization$Foo["baz"]->tools.jackson.databind.exc.TestExceptionHandlingWithDefaultDeserialization$Bar["baz"]>
[ERROR]   TypedArraySerTest.testListWithPolymorphic:87 expected: <{"beans":[{"@type":"bean","x":0}]}> but was: <{"beans":[{"@type":"bean"}]}>
[ERROR] Errors: 
[ERROR]   CurrentValueDeser4184Test.testCurrentValue4184EmptyPojo:92 » MismatchedInput currentValue() of wrong type, not User but: tools.jackson.databind.ser.filter.CurrentValueDeser4184Test$Role
 at [Source: (String)"{"role": {}, "type": {"value":1}}"; line: 1, column: 22] (through reference chain: tools.jackson.databind.ser.filter.CurrentValueDeser4184Test$User["type"])
[ERROR]   TestPOJOAsArray.testUnknownExtraProp:309 » MismatchedInput Unexpected token (`JsonToken.VALUE_FALSE`), expected `JsonToken.END_ARRAY`: Unexpected JSON values; expected at most 4 properties (in JSON Array)
 at [Source: (String)"{"value":[true,"Foobar",42,13, false]}"; line: 1, column: 32] (through reference chain: tools.jackson.databind.struct.TestPOJOAsArray$PojoAsArrayWrapper[""])

[cowtowncoder]

Down to just 2 failures:

 Error:  Failures: 
Error:    CollectionDeserTest.testArrayIndexForExceptions:298 expected: <keys> but was: <>
Error:    TestExceptionHandlingWithDefaultDeserialization.testShouldThrowExceptionWithPathReference:58 expected: <tools.jackson.databind.exc.TestExceptionHandlingWithDefaultDeserialization$Foo["bar"]->tools.jackson.databind.exc.TestExceptionHandlingWithDefaultDeserialization$Bar["baz"]> but was: <tools.jackson.databind.exc.TestExceptionHandlingWithDefaultDeserialization$Foo["baz"]->tools.jackson.databind.exc.TestExceptionHandlingWithDefaultDeserialization$Bar["baz"]>
[INFO] 
Error:  Tests run: 3484, Failures: 2, Errors: 0, Skipped: 0

[sdeleuze]

@cowtowncoder Could you please apply this change to the upcoming 3.0.0-rc4 as Spring does not customize the builder anymore and the old default breaks Spring use cases?

[JooHyukKim]

There is on-going PR to resolve this issue #4885.
But we faced concerns wrt performance 🤔

[JooHyukKim]

Speaking of which, @sdeleuze has there been any reports about performance issue when set to false as default?
If there would be issue, frameworks like Spring/Quarks would likely be the one to identify this issue, due to wide usage.

[JooHyukKim]

@cowtowncoder Is there specific usage pattern that could possibly be bottleneck?

[sdeleuze]

false is the default on Spring for years, with no performance issue reported. I would add that we have removed our custom Jackson builder from Spring Framework 7 to leverage just Jackson 3 default builders and configuration based on the fact that the switch to fasle was confirmed in FasterXML/jackson-future-ideas#75, so this is a pretty big deal for us.

[cowtowncoder]

Performance concern is mine: the problem is that with default setting of include if no view definition specified, and no View activated, the whole View processing pathway is ignored during serialization.
But with default of not including, View processing is always performed even if no active View is set.
This seemed to have something like 10% slowdown for serialization case on jackson-benchmarks.

Although I don't consider -10% performance penalty huge in general (it really isn't); since it is across all users, it'd be unfortunate thing to add.

Maybe I'll find time try if I can reproduce my findings and think about it this a bit more.

Aside from this, one thing I would strongly recommend Spring team -- regardless of how this setting goes -- is to retain capability of being able to override defaults. Just in case; it is quite likely defaults for Spring will diverge ever so slightly.

[sdeleuze]

But with default of not including, View processing is always performed even if no active View is set.

There is maybe a misunderstanding or something I miss, but Spring ask should not have any performance impact when no view is activated. Our ask is that when writerWithView(MyView.class) or readerWithView(MyView.class) is invoked, properties not annotated with a view are not included. This is the use case I described in 2014 when I introduced related support in Spring which as become extremely popular and natural to end users.

I struggle to understand how and why the "view processing code path" with its 10% performance penalty should be involved when writerWithView(MyView.class) or readerWithView(MyView.class) are not invoked. Unless I miss something, it should not be the case. MapperFeature.DEFAULT_VIEW_INCLUSION should have an impact only when a view is activated, when it is not the case, the code path should be the non-view one without the 10% performance penalty.

Could you please elaborate on why MapperFeature.DEFAULT_VIEW_INCLUSION has currently an impact when no view is activated? Do you think we could target the behavior I describe to get the best of the 2 worlds: natural JSON view handling by only including properties annotated with the active view and no impact + fast code path if no view is activated?

[cowtowncoder]

@sdeleuze I can understand that it'd seem no processing would occur if no View is defined, but there indeed is separate code path with more processing exactly even if no active view is specified but DEFAULT_VIEW_INCLUSION is false (but not when it is true).
I realized this when looking into test failures when changing defaults, and did some performance benchmarking as a result.

And I thought this was specifically for serialization.

Ideally there wouldn't be difference of course, but I don't think that is easy to resolve. I'll dig up a pointer if you want to check it yourself.

[cowtowncoder]

Ok. So figuring this out is trickier than I remember but basically entry points are:

• For deserialization BeanDeserializerBuilder, line 571: https://github.com/FasterXML/jackson-databind/blob/3.x/src/main/java/tools/jackson/databind/deser/BeanDeserializerBuilder.java#L571 -- _anyViews will return true because now even if no View annotations are associated with properties, check must be made (if default inclusion is true, then no annotations means all would be included)
• For serialization; BeanSerializerFactory, line 659: https://github.com/FasterXML/jackson-databind/blob/3.x/src/main/java/tools/jackson/databind/ser/BeanSerializerFactory.java#L659 -- similarly, cannot skip processing

but from thereon, processing needed in BeanSerializer[Base] and BeanDeserializer does not appear complicated.

I think I do need to run jackson-benchmarks cases locally to double-check I can actually reproduce what I think was measurable difference.
In best case I cannot see meaningful difference.

[cowtowncoder]

Ok: after re-running combinations of:

• ser/deser
• regular/afterburner
• json/smile

over https://github.com/FasterXML/jackson-benchmarks/, I think TL;DNR is -- I no longer have concerns.

It looks like:

1. For serialization there is NO general measurable difference: the only thing I can see is up to 2% lower throughput for JSON + Afterburner + Ser, and that's probably within margin-of-error of tests
2. For deserialization I see some very minor differences,2-5% lower throughput, for all permutations. Highest (5%) for JSON + Afterburner + Deser, lowest for Smile + regular + Deser (2%).

although it is possible there might be bigger difference for some usage it seems unlikely and I am not going to block this change for dinky 2% performance degradation which is barely measurable.

Not sure where I thought I see up to 10% difference earlier (possibly on 2.x? or maybe JDK 8 had bigger difference) but it is not there in 3.x branch for json/smile content.

I will proceed merging this later today for 3.0.0-rc4 @sdeleuze .

[cowtowncoder]

PR merged, will be in 3.0.0-rc4.

[JooHyukKim]

Great job @cowtowncoder !