[GitHub] trafficserver pull request #965: TS-4532: Static type checking for time.

[SolidWallOfCode]

Github user SolidWallOfCode closed the pull request at:

https://github.com/apache/trafficserver/pull/965


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #965: TS-4532: Static type checking for time.

[SolidWallOfCode]

Github user SolidWallOfCode commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/965#discussion_r77548123
  
--- Diff: iocore/eventsystem/PQ-List.cc ---
@@ -22,18 +22,31 @@
  */
 
 #include "P_EventSystem.h"
+typedef std::chrono::duration > bucket_time;
--- End diff --

I think this is wrong. The ratio should be something like ``<5,1000>`` not 
``<1, 5000>``.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #965: TS-4532: Static type checking for time.

[SolidWallOfCode]

Github user SolidWallOfCode commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/965#discussion_r77541569
  
--- Diff: iocore/eventsystem/UnixEThread.cc ---
@@ -149,10 +149,10 @@ EThread::process_event(Event *e, int calling_code)
 ink_assert(!e->in_the_priority_queue);
 ink_assert(c_temp == e->continuation);
 MUTEX_RELEASE(lock);
-if (e->period) {
+if (e->period.count()) {
   if (!e->in_the_prot_queue && !e->in_the_priority_queue) {
-if (e->period < 0)
-  e->timeout_at = e->period;
+if (e->period.count() < 0)
+  e->timeout_at = ts_hrtick::min() + e->period;
--- End diff --

Change to ``e->timeout_at = TS_HRTICK_ZERO + e->period``.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #965: TS-4532: Static type checking for time.

[SolidWallOfCode]

Github user SolidWallOfCode commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/965#discussion_r77541520
  
--- Diff: iocore/eventsystem/UnixEThread.cc ---
@@ -149,10 +149,10 @@ EThread::process_event(Event *e, int calling_code)
 ink_assert(!e->in_the_priority_queue);
 ink_assert(c_temp == e->continuation);
 MUTEX_RELEASE(lock);
-if (e->period) {
+if (e->period.count()) {
   if (!e->in_the_prot_queue && !e->in_the_priority_queue) {
-if (e->period < 0)
-  e->timeout_at = e->period;
+if (e->period.count() < 0)
--- End diff --

Or ``e->period < e->period.zero()`` ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #965: TS-4532: Static type checking for time.

[SolidWallOfCode]

Github user SolidWallOfCode commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/965#discussion_r77540649
  
--- Diff: iocore/eventsystem/I_Thread.h ---
@@ -168,21 +168,21 @@ class Thread
 
   @note This also updates the cached time.
   */
-  static ink_hrtime get_hrtime_updated();
+  static ts_hrtick get_hrtime_updated();
 };
 
 extern Thread *this_thread();
 
-TS_INLINE ink_hrtime
+TS_INLINE ts_hrtick
 Thread::get_hrtime()
 {
   return cur_time;
 }
 
-TS_INLINE ink_hrtime
+TS_INLINE ts_hrtick
 Thread::get_hrtime_updated()
 {
-  return cur_time = ink_get_hrtime_internal();
+  return cur_time = std::chrono::high_resolution_clock::now();
--- End diff --

Wrong. Should be ``ts_hrtick::clock::now()``.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #965: TS-4532: Static type checking for time.

[SolidWallOfCode]

Github user SolidWallOfCode commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/965#discussion_r77540451
  
--- Diff: iocore/eventsystem/I_Lock.h ---
@@ -241,7 +241,7 @@ class ProxyMutex : public RefCountObj
 thread_holding  = NULL;
 nthread_holding = 0;
 #ifdef DEBUG
-hold_time = 0;
+hold_time = hold_time.min();
--- End diff --

SHould be ``hold_time = TS_HRTICK_ZERO;``.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #965: TS-4532: Static type checking for time.

[SolidWallOfCode]

Github user SolidWallOfCode commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/965#discussion_r77540321
  
--- Diff: iocore/eventsystem/I_Lock.h ---
@@ -28,7 +28,7 @@
 #include "ts/Diags.h"
 #include "I_Thread.h"
 
-#define MAX_LOCK_TIME HRTIME_MSECONDS(200)
+static const ts_microseconds MAX_LOCK_TIME(200);
--- End diff --

This could also be
{{code}}
static const ts_nanoseconds MAX_LOCK_TIME(ts_microseconds(200));
{{code}}
That style is used elsewhere, I'm not sure which is better. Denoting this 
value in nanoseconds avoids conversions later.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #965: TS-4532: Static type checking for time.

[SolidWallOfCode]

GitHub user SolidWallOfCode opened a pull request:

https://github.com/apache/trafficserver/pull/965

TS-4532: Static type checking for time.

Rough first pass just for iocore/eventsystem to get a feel for the changes.

THIS WILL NOT RUN - it's just an experiment to be verified before further 
work is done.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/SolidWallOfCode/trafficserver ts-4532

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/trafficserver/pull/965.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #965


commit c96fdecad810ee680475377faf9fca2bdbf817b4
Author: Alan M. Carroll 
Date:   2016-09-05T15:58:14Z

TS-4532: Static type checking for time.
Rough first pass just for iocore/eventsystem to get a feel for the changes.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---