Skip to content

5.2.x java time session and deadline#310

Closed
garydgregory wants to merge 2 commits into
apache:masterfrom
garydgregory:5.2.x_java_time_session_and_deadline
Closed

5.2.x java time session and deadline#310
garydgregory wants to merge 2 commits into
apache:masterfrom
garydgregory:5.2.x_java_time_session_and_deadline

Conversation

@garydgregory
Copy link
Copy Markdown
Member

Use Java 8's java.time package in sessions and deadlines.

Copy link
Copy Markdown
Member

@ok2c ok2c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@garydgregory IOSession is a low level transport class. There are absolutely no benefit of using Instant for internal time representation in it. I see no point changing it. Deadline changes look good to me.

@ok2c
Copy link
Copy Markdown
Member

ok2c commented Oct 29, 2021

@garydgregory I also looked at the Instant#now implementation. It basically uses System.currentTimeMillis() internally. So, I also see no benefit of using it in low level classes.

@garydgregory
Copy link
Copy Markdown
Member Author

@garydgregory I also looked at the Instant#now implementation. It basically uses System.currentTimeMillis() internally. So, I also see no benefit of using it in low level classes.

Newer versions of Java (like 11) provide nanosecond resolution. Not everyone is stuck on Java 8.

@ok2c
Copy link
Copy Markdown
Member

ok2c commented Oct 29, 2021

Not everyone is stuck on Java 8.
@garydgregory But we are.

@garydgregory
Copy link
Copy Markdown
Member Author

Not everyone is stuck on Java 8.
@garydgregory But we are.

Uh? Anyone can run our jar files on Java 11 and 17 and get the benefits.

@ok2c
Copy link
Copy Markdown
Member

ok2c commented Oct 29, 2021

get the benefits.

@garydgregory What would those be? Nanosecond time resolution for timeout calculation with 1s precision?

@garydgregory
Copy link
Copy Markdown
Member Author

garydgregory commented Oct 29, 2021

get the benefits.

@garydgregory What would those be? Nanosecond time resolution for timeout calculation with 1s precision?

The most important aspect of this type of change is type safety. On Java 8, we can use objects like Instant and Duration instead of a mix of int and long which appear unscaled and are subject to underflow, overflow, and wrap around behavior. Not to mention that sometimes an argument is expressed in seconds, sometimes it's in milliseconds, a sure recipe for bugs.

@ok2c
Copy link
Copy Markdown
Member

ok2c commented Oct 29, 2021

get the benefits.

@garydgregory What would those be? Nanosecond time resolution for timeout calculation with 1s precision?

The most important aspect of this type of change is type safety. On Java 8, we can use objects like Instant and Duration instead of a mix of int and long which appear unscaled and are subject to underflow, overflow, and wrap around behavior. Not to mention that sometimes an argument is expressed in seconds, sometimes it's in milliseconds, a sure recipe for bugs.

@garydgregory This is a legitimate reason but not for low level classes where memory footprint matters more. If have spare cycles better invest them into porting high level APIs (such those in the cookie and cache packages) from Date to Instance.

@garydgregory
Copy link
Copy Markdown
Member Author

get the benefits.

@garydgregory What would those be? Nanosecond time resolution for timeout calculation with 1s precision?

The most important aspect of this type of change is type safety. On Java 8, we can use objects like Instant and Duration instead of a mix of int and long which appear unscaled and are subject to underflow, overflow, and wrap around behavior. Not to mention that sometimes an argument is expressed in seconds, sometimes it's in milliseconds, a sure recipe for bugs.

@garydgregory This is a legitimate reason but not for low level classes where memory footprint matters more. If have spare cycles better invest them into porting high level APIs (such those in the cookie and cache packages) from Date to Instance.

From where I stand, consistency is important, as is the use of the better abstractions. I don't see evidence of memory footprint issues. I've not seen memory issues in this department in the past. There are lots of objects coming and going in a Java app, even a trivial one, and Java is getting better and better at managing memory on and off heap for object allocation. I brief, I don't see a real issue.

@ok2c ok2c force-pushed the master branch 2 times, most recently from c4a2881 to fb864e2 Compare December 19, 2021 11:08
@ok2c ok2c force-pushed the master branch 3 times, most recently from a60a060 to 8561cbd Compare February 2, 2022 18:25
@ok2c ok2c force-pushed the master branch 3 times, most recently from 76dc849 to 912683c Compare October 2, 2022 14:32
@ok2c ok2c force-pushed the master branch 2 times, most recently from fa9f5f8 to f1bb865 Compare October 19, 2022 11:55
@ok2c ok2c force-pushed the master branch 3 times, most recently from 1d68d86 to e154468 Compare November 2, 2022 13:34
Copy link
Copy Markdown
Member

@michael-o michael-o left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @ok2c Instant gives here zero benefit compared with long.

@ok2c ok2c closed this Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants