Improves validation performance of types defined in the spec

[popematt]

Issue #, if available:

None

Description of changes:

I discovered recently that the types defined in the spec are (subjectively) slow-ish for validation. I tried an alternative implementation, and ended with a pretty good improvement. Serendipitously, I discovered that this fixes #284 and partially #169.

This change:

• Completely rewrites TypeBuiltin
• Adds BuiltInTypes, an object for holding all TypeBuiltin instances
• Removes, SchemaCore, TypeBuiltInImpl, TypeIon, and TypeCore.
• Adds tests for TypeNullable

My informal performance testing revealed some really big improvements. I chose some representative spec types that corresponded to 0, 1, "many", and "all" Ion types, and I created test cases that used the spec types to validate some sample data (one value of each Ion type except for clob and sexp). These results were measured by running warmups until the time was stable, using an "autorange" approach to determine the number of invocations in each sample, and then collecting and averaging the ops/s for 30 samples.

Type	Before (ops/s)	After (ops/s)	Difference
nothing	134126	14928349	+11030%
any	153279	44165490	+28713%
$any	135234	57456489	+42386%
number	657438	11096099	+1587%
$number	620417	12594188	+1930%
int	2043462	6880997	+236%
$int	1957613	9165562	+368%

I tested with both ISL 1.0 and 2.0, and the results were basically the same, but this is especially impactful for ISL 1.0, because any user type definition that does not have the type constraint has an implied type: any.

Edit:

This PR also removes some unnecessary noise in the violations messages. E.g. when testing null.symbol against type any, the current message is a lot of noise, but the important thing is "not applicable for null values", which is rather cryptic.

Validation failed:
- expected type any
  - value matches none of the types
    - expected type blob, found symbol
    - expected type bool, found symbol
    - expected type clob, found symbol
    - expected type decimal, found symbol
    - expected type document, found symbol
    - expected type float, found symbol
    - expected type int, found symbol
    - expected type string, found symbol
    - not applicable for null values
    - expected type timestamp, found symbol
    - expected type list, found symbol
    - expected type sexp, found symbol
    - expected type struct, found symbol

The new violation message is a one liner that mentions the null-ness and the Ion type of the value.

Validation failed:
- expected type any, found null symbol

Related PRs in ion-schema, ion-schema-tests, ion-schema-schemas:

None.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[codecov]

Codecov Report

Patch coverage is 85.91% of modified lines.

Files Changed	Coverage
...n/kotlin/com/amazon/ionschema/internal/TypeImpl.kt	0.00%
...lin/com/amazon/ionschema/internal/TypeReference.kt	66.66%
...in/com/amazon/ionschema/internal/SchemaImpl_1_0.kt	75.00%
...otlin/com/amazon/ionschema/internal/TypeBuiltin.kt	88.33%
...m/amazon/ionschema/internal/IonSchemaSystemImpl.kt	100.00%
...in/com/amazon/ionschema/internal/SchemaImpl_2_0.kt	100.00%
...tlin/com/amazon/ionschema/internal/TypeNullable.kt	100.00%

📢 Thoughts on this report? Let us know!.

[desaikd]

(Question) Why was this removed? I couldn't find any changes in Violation structure for this PR.

[popematt]

The old way of creating the spec-defined types involved reading an Ion struct and then extracting the symbol from the struct. This meant that the symbols had the field name "type". See

ion-schema-kotlin/ion-schema/src/main/kotlin/com/amazon/ionschema/internal/SchemaCore.kt

Lines 86 to 118 in a3fabe9

	private const val CORE_TYPES =
	"""
	{ type: blob }
	{ type: bool }
	{ type: clob }
	{ type: decimal }
	{ type: document }
	{ type: float }
	{ type: int }
	{ type: string }
	{ type: symbol }
	{ type: timestamp }
	{ type: list }
	{ type: sexp }
	{ type: struct }
	"""
	private const val ION_TYPES =
	"""
	{ type: ${'$'}blob }
	{ type: ${'$'}bool }
	{ type: ${'$'}clob }
	{ type: ${'$'}decimal }
	{ type: ${'$'}float }
	{ type: ${'$'}int }
	{ type: ${'$'}null }
	{ type: ${'$'}string }
	{ type: ${'$'}symbol }
	{ type: ${'$'}timestamp }
	{ type: ${'$'}list }
	{ type: ${'$'}sexp }
	{ type: ${'$'}struct }
	"""

Now we create the symbol directly instead, which means that the symbol no longer has a field name. (This whole thing is a weird consequence of the unfortunate decision that an IonValue knows its own field name.) Anyway, this lack of field name causes no difference in the violation messages, and the violation constraint field name is not actually used anywhere in the library.