[ 
https://issues.apache.org/jira/browse/GEODE-8601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17311277#comment-17311277
 ] 

ASF GitHub Bot commented on GEODE-8601:
---------------------------------------

gaussianrecurrence commented on a change in pull request #678:
URL: https://github.com/apache/geode-native/pull/678#discussion_r603896118



##########
File path: cppcache/include/geode/CacheStatistics.hpp
##########
@@ -47,9 +47,9 @@ class LocalRegion;
  */
 class APACHE_GEODE_EXPORT CacheStatistics {
  public:
-  typedef std::chrono::system_clock::time_point time_point;
+  using time_point = std::chrono::steady_clock::time_point;
 
-  CacheStatistics() : m_lastAccessTime(0), m_lastModifiedTime(0) {}
+  CacheStatistics() = default;

Review comment:
       > This yet another awesome move away from ACE. I can appreciate the work 
put into this because I have tried and failed about 3 times myself. I need a 
bit more time to review the remaining source but right out the gate I am 
worried about the `clock` usage and change from `system_clock` to 
`steady_clock`. While I prefer we work internally in terms of `steady_clock` if 
any of the time is exchanged between processes or persisted it needs to be in 
`system_clock`.
   
   After looking into it, CacheStatistics expects that an 
system_clock::time_point. This might potentially be used by any NC user to get 
modification and access time. But internally this stats are used to calculate 
elapsed time rather than a fixed time point. So what I did is to internally 
store it as steady_clock::time_point and public getters converts it to 
system_clock::timepoint by calculating the delta between both clocks.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Replace ACE ExpiryTaskManager impl by boost::asio one
> -----------------------------------------------------
>
>                 Key: GEODE-8601
>                 URL: https://issues.apache.org/jira/browse/GEODE-8601
>             Project: Geode
>          Issue Type: Improvement
>          Components: native client
>            Reporter: Mario Salazar de Torres
>            Assignee: Mario Salazar de Torres
>            Priority: Major
>              Labels: obliterate-ace, pull-request-available
>
> *AS A* native client contributor
> *I WOULD LIKE* to replace ACE implementation of ExpiryTaskManager for a 
> boost::asio one
> *SO THAT* all issues related to it disappear, the implementation is more 
> simple and move away from ACE as it's the project policy for a while.
> —
> *Additional information*
> Current implementation of ExpiryTaskManager is causing issue GEODE-8535 and 
> some other related, not yet reported.
> Also, as I am aware the project is moving away from ACE, so I'd say this is 
> the perfect oportunity to replace ExpiryTaskManager implementation.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to