Skip to content

Conversation

@danobi
Copy link
Member

@danobi danobi commented Apr 13, 2015

Moved ink_get_hrtime() and ink_get_based_hrtime() into the Thread.cc class as opposed to them being an extern function in P_Thread.h. Also made them static functions. Fixed the rest of the code base to refer to new functions.
Running 'make test' seemed to work.

Please let me know if anything is wrong, I'd be happy to correct any errors. Also please let me know if this change actually makes sense, because with my limited knowledge this was the only solution I could come up with.

danobi added 2 commits April 12, 2015 23:39
Tested: Debian Jessie Testing
Author: Daniel Xu <dlxu2@illinois.edu>

Moved ink_get_hrtime() and ink_get_based_hrtime()
into the Thread.cc class and made them static
functions as well to support object free calling.
Also changed all references in code base to these
two functions to refer to Thread::ink_get_...
instead.
Tested: Debian Jessie Testing
Author: Daniel Xu <dlxu2@illinois.edu>

Moved ink_get_hrtime() and ink_get_based_hrtime()
into the Thread.cc class and made them static
functions as well to support object free calling.
Also changed all references in code base to these
two functions to refer to Thread::ink_get_...
instead. Also fixed my own tab spacing issue.
@jpeach
Copy link
Contributor

jpeach commented Apr 13, 2015

Doe we even need the new Thread APIs? Can we just use Thread::cur_time directly?

@SolidWallOfCode
Copy link
Member

Isolation and modularity? It seems better to discuss this on the original bug - it's not really a problem with the pull request.

@SolidWallOfCode
Copy link
Member

Sorry for the delay, I got distracted. I think it's OK except the method name should be just "get_hrtime()", there's no need for the "ink_" prefix if it's a method.

Remove the 'ink' prefix on {get,get_based}_hrtime() functions
@danobi
Copy link
Member Author

danobi commented Jun 9, 2015

Fixed. However, I am unsure how to handle all the merge conflicts.

@SolidWallOfCode
Copy link
Member

I think the most likely cause is the code base was reformatted and you're being hit by that. You need to reformat you code base to match before trying to rebase. You can find them here - https://bintray.com/apache/trafficserver/clang-format-tools/view

Daniel Xu added 2 commits June 11, 2015 15:55
Fix the merge conflicts.

Conflicts:
	proxy/Update.cc
	proxy/http/HttpSM.cc
Code was added in some source files that used the
'old' get_hrtime() functions after my initial refactoring.
After the merge, some changes needed to be made.
@danobi
Copy link
Member Author

danobi commented Jun 11, 2015

Fixed the merge conflicts. Also updated any other uses of ink_get_hrtime() / ink_get_based_hrtime() that occurred during the life of this PR.

There's quite a few commits now, let me know if this should all be squashed.

Merge remote branch 'upstream/master'

Conflicts:
	proxy/ICP.cc
@bryancall
Copy link
Contributor

@asfgit asfgit closed this in 738d9ed Jun 23, 2015
SolidWallOfCode pushed a commit to SolidWallOfCode/trafficserver that referenced this pull request May 7, 2016
This closes apache#185.

Conflicts:
	iocore/net/P_UnixNetVConnection.h
	iocore/net/UnixNet.cc
	proxy/ICP.cc
	proxy/http/HttpSM.cc
bneradt pushed a commit to bneradt/trafficserver that referenced this pull request Nov 19, 2020
Fix rare SSN/TXN Start/Close Hook misorderings (apache#6364)
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