FasterXML / FasterXML/woodstox

Reconsider `TextBuilder` initial size/growth rate

Open
#259 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
259
Forks
90
Avg merge
16h 52m
Merged PRs (30d)
2

Description

`BasicStreamReader.parseAttrValue()` is one of the hotspots when unmarshalling XML.

I investigated that one of the pain points is buffer resizing in `TextBuilder`:
* `AttributeCollector` holds 2 `TextBuilder`s, one for the namespaces (`mNamespaceBuilder`) and one for the other attributes (`mValueBuilder`).
* The initial buffer size of `mNamespaceBuilder` is `EXP_NS_COUNT(==6) << 4` which is 96
* The initial buffer size of `mValueBuilder` is `EXP_ATTR_COUNT(==12) << 4` which is 192, but this size is clamped to `TextBuilder.MAX_LEN` which is 120
* On `resize()`, the buffer grows by only 50%

Complex XML files easily outgrow the initial buffers. Take e.g. the namespaces from some [WS-Discovery probe message](https://learn.microsoft.com/en-us/windows/win32/wsdapi/probe-message):
```
https://www.w3.org/2003/05/soap-envelope
https://schemas.xmlsoap.org/ws/2004/08/addressing
https://schemas.xmlsoap.org/ws/2005/04/discovery
https://schemas.xmlsoap.org/ws/2006/02/devprof
```
These are already more than 180 chars, shortest one 41 chars.

In our use-case, the namespaces sometimes even exceed 700 chars...

Concluding, I would argue that:
* the initial buffer size 96 for `mNamespaceBuilder` is on the lower side and will probably be exceeded once 3 or 4 namespaces are used
* the clamping to 120 chars for the `mValueBuilder` is understandable, but a bit weird (because 192 is reduced to 120 on every instantiation)
* the growth-factor of 50% is unusually careful, given that `StringBuilder`/`StringBuffer` typically grow by `(oldCapacity * 2) + 2`

I suggest to at least change the growth in `TextBuilder.resize()` from
```java
int addition = oldLen >> 1; // Grow by 50%
// 96 -> 144 -> 216 -> 324 -> 486, -> 729 -> 1093 bytes
```
to
```java
int addition = oldLen; // Grow by 100%
// 96 -> 192 -> 384 -> 768 -> 1536 bytes
```
This shouldn't hurt if not needed. What do you think? Maybe also increase initial size(s)? Or make it easier to fine-tune `TextBuilder` if needed?

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.