Re: [LOG4J2-3672] Time zone parsing in `FastDateParser`

2023-11-03 Thread Volkan Yazıcı
Done. On Thu, Nov 2, 2023 at 6:58 PM Piotr P. Karwasz wrote: > Hi Volkan, > > On Thu, 2 Nov 2023 at 18:50, Volkan Yazıcı wrote: > > > > I guess the safest alternative would be to lazily initialize the > > `FastDateFormat#parser` field. > > I am happy we agree on the right solution, respectful o

Re: [LOG4J2-3672] Time zone parsing in `FastDateParser`

2023-11-02 Thread Volkan Yazıcı
I guess the safest alternative would be to lazily initialize the `FastDateFormat#parser` field. Nevertheless, I do not want to miss this deprecation unicorn, and hence, deleted the code and some tests

Re: [LOG4J2-3672] Time zone parsing in `FastDateParser`

2023-11-02 Thread Piotr P. Karwasz
Hi Volkan, On Thu, 2 Nov 2023 at 18:50, Volkan Yazıcı wrote: > > I guess the safest alternative would be to lazily initialize the > `FastDateFormat#parser` field. I am happy we agree on the right solution, respectful of the public API! Now you have my blessing to shred those unused classes to pi

Re: [LOG4J2-3672] Time zone parsing in `FastDateParser`

2023-11-02 Thread Piotr P. Karwasz
Hi Volkan, On Thu, 2 Nov 2023 at 16:16, Volkan Yazıcı wrote: > I deleted all date parsing functionality in Log4j[1] and the compilation... > [drum roll...] succeeded. I am inclined to commit this breaking change. We > can refer users who were previously using Log4j to parse dates (ugh!) to > Comm

Re: [LOG4J2-3672] Time zone parsing in `FastDateParser`

2023-11-02 Thread Ralph Goers
+1 to this change. Ralph > On Nov 2, 2023, at 8:16 AM, Volkan Yazıcı wrote: > > I deleted all date parsing functionality in Log4j[1] and the compilation... > [drum roll...] succeeded. I am inclined to commit this breaking change. We > can refer users who were previously using Log4j to parse dat

Re: [LOG4J2-3672] Time zone parsing in `FastDateParser`

2023-11-02 Thread Volkan Yazıcı
I deleted all date parsing functionality in Log4j[1] and the compilation... [drum roll...] succeeded. I am inclined to commit this breaking change. We can refer users who were previously using Log4j to parse dates (ugh!) to Commons Lang. The API is identical, they simply need to use a separate pack

Re: [LOG4J2-3672] Time zone parsing in `FastDateParser`

2023-11-02 Thread Volkan Yazıcı
*Conclusion:* I will make time zone parsing in `FastDateParser` *slow* to avoid `getZoneStrings()`. *Remark:* JTL has the most versatile `Instant` formatter combining `FixedDateFormat`, `FastDateFormat`, and `DateTimeFormatter`. See `InstantFormatter` for why and how. I will later hoist this up to

Re: [LOG4J2-3672] Time zone parsing in `FastDateParser`

2023-11-01 Thread Piotr P. Karwasz
Hi Ralph On Wed, 1 Nov 2023 at 23:53, Ralph Goers wrote: > > On Nov 1, 2023, at 3:33 PM, Piotr P. Karwasz > > wrote: > > Actually we don't need a fast formatter either. Except for the rolling > > file manager that can require a date from the past, the layouts format > > timestamps close to 'now

Re: [LOG4J2-3672] Time zone parsing in `FastDateParser`

2023-11-01 Thread Ralph Goers
> On Nov 1, 2023, at 3:33 PM, Piotr P. Karwasz wrote: > > Hi Volkan, > > On Wed, 1 Nov 2023 at 12:07, Volkan Yazıcı wrote: >> Curious: *In the context of logging, does `FastDateParser` need to be fast >> while parsing?* We certainly need to *"format"* the instant fast. Though do >> we really

Re: [LOG4J2-3672] Time zone parsing in `FastDateParser`

2023-11-01 Thread Piotr P. Karwasz
Hi Volkan, On Wed, 1 Nov 2023 at 12:07, Volkan Yazıcı wrote: > Curious: *In the context of logging, does `FastDateParser` need to be fast > while parsing?* We certainly need to *"format"* the instant fast. Though do > we really need to *"parse"* it fast too? No we don't. In Commons Lang the pars

Re: [LOG4J2-3672] Time zone parsing in `FastDateParser`

2023-11-01 Thread Matt Sicker
I can’t think of any reason why we’d need fast date parsing. Unless it’s relevant in the rolling file appender family? > On Nov 1, 2023, at 6:07 AM, Volkan Yazıcı wrote: > > As reported in LOG4J2-3672 > , `FastDateParser`[1] > contains `DateFo

[LOG4J2-3672] Time zone parsing in `FastDateParser`

2023-11-01 Thread Volkan Yazıcı
As reported in LOG4J2-3672 , `FastDateParser`[1] contains `DateFormatSymbols#getZoneStrings()`, which causes initialization and caching of all time zones, resulting in a ~3MB heap overhead on x86_64. The reporter also provided the PR #1848