Hi,
see inline
Am 08.08.23 um 19:30 schrieb Daniel Gredler:
Hi,
I think I've said this before, but thanks again for such a great library!
I'm thinking about submitting a patch to improve PDFBox, but wanted to get
the team's thoughts first.
What I've seen is that sometimes under very heavy load across multiple
threads, PDF creation stalls due to lock contention, because
`TTFDataStream.readInternationalDate()` uses
`TimeZone.getTimeZone(String)`, which is synchronized. I assume that the
inverse operation,
`TTFSubsetter.writeLongDateTime(DataOutputStream,Calendar)`, has a similar
issue, though I have not seen it myself.
Good catch. PDFBox is not supposed to be thread-safe, but it is always a
good idea to eliminate obvious issues.
If these classes were using the newer `java.time` APIs, they could simply
use the `ZoneOffset.UTC` constant... but they are using the older
`Calendar` API (and exposing this fact), so `TimeZone` must be used. So a
few questions from my side:
Is there a plan to move off of the older time APIs and onto the newer
`java.time` APIs, perhaps as part of version 3.0? (e.g. `Calendar` ->
`ZonedDateTime`) Either way, I'm pretty sure such a breaking change is not
contemplated for the 2.x release series, correct?
No plan so far. There were some discussions in the past but other things
very more important or more interesting ;-) There were some small
changes in the trunk.
We already released a beta of 3.0 which implies a stable api so that we
must not change that. Furthermore I'm planing to cut the final release
next Monday and I guess there isn't enough time to do such changes at all.
2.x is a no go as well, as it relies on java 6 and the java.time api
requires java 8
`TimeZone` is not technically thread-safe, but there only a couple of
rarely-used dangerous mutators, AFAIK (`setRawOffset(int)`,
`setID(String)`). Would it be OK to keep a static final UTC `TimeZone` and
just reuse that, even though `TimeZone` is only 95% immutable?
I concur with Tilman, this would be a small but good solution for 3.0
and 2.x as well
Let's discuss the topic after releasing 3.0. Maybe it is a good idea to
target 4.x for such a change.
If not, what about keeping a static final UTC `TimeZone` constant and
creating a clone each time it is needed, to avoid the synchronization?
(`Calendar.getTimeZone()` makes liberal use of `clone()`, for example).
If that doesn't work, what about subclassing `TimeZone` and overriding the
mutators with methods that just throw `UnsupportedOperationException`s, and
using the subclass for the static final UTC constant? Commons Lang 3 does
something similar with `GmtTimeZone` and the `FastTimeZone` utility class
here:
https://github.com/apache/commons-lang/blob/bf5865ae915ececcdbfa7a473b0d708e3e235bcf/src/main/java/org/apache/commons/lang3/time/FastTimeZone.java#L38
Thanks for the feedback!
Daniel
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org
For additional commands, e-mail: dev-h...@pdfbox.apache.org
