Skip to content
This repository was archived by the owner on Oct 21, 2024. It is now read-only.

Conversation

@ip1996
Copy link
Collaborator

@ip1996 ip1996 commented Dec 29, 2020

No description provided.

@gerazo
Copy link
Owner

gerazo commented Feb 12, 2021

@ip1996 This looks perfect, but I can't test it. Is it working perfectly, or is something still missing? I am asking in order to know what to write into the documentation.

@ip1996
Copy link
Collaborator Author

ip1996 commented Feb 14, 2021

It was:
before_build_result.txt
However, I merged master into my branch, and the new tests are failing:
build_result.txt
The most affected is the SenderLoggerTest, where the GetTopSender is usually NULL, plus in Scheduler.SchedulerPipeLineTest the new timestamp assertion in ChannelImpl keeps failing. I tried to debug the issues, but I didn't resolve them yet.

@gerazo
Copy link
Owner

gerazo commented Feb 15, 2021

On the t - packet->timestamp_ >= 0 assertion, I think that it has something to do with the clock we are using. MSVC always had crappy clock implementations but it changes from version to version: https://stackoverflow.com/questions/16299029/resolution-of-stdchronohigh-resolution-clock-doesnt-correspond-to-measureme
However, there is a general mention of the fact that the high:_resolution_clock we are using should be replaced with a steady_clock instead: (at the end note: https://en.cppreference.com/w/cpp/chrono/high_resolution_clock )
So I guess changing highresclock to the steady can be a first try. (Although I have checked that the two occurences of clock calls between the time goes negative is done from the same thread... but it is possible that somehow the thread jumps to an other CPU during this execution and because MSVC2015 uses QueryPerformanceCounters which is stupid again because it asks CPU cycle counters, these can differ between CPU cores and because of this, we have a negative time. Although it is possible that the original type in which we get back the time is so small that it wraps around...)

@gerazo
Copy link
Owner

gerazo commented Feb 15, 2021

In case of the 2nd problem (which is release only, so even nastier), either thread_local is not working correctly or TrackSender does not register the thread name. Because we have tests for the former, I would start debugging TrackSender... what it gets as a thread name. There is synchronization involved and everything, so it can be something fishy with the MSVC optimization.
And one general question: What MSVC version are you using?
It would be super to catch these problems.

@gerazo
Copy link
Owner

gerazo commented Mar 9, 2021

@ip1996 will you have time for this? Because if not, it is still a useful upgrade what we could merge anyway. In this case, we can put this into an issue for later.

@ip1996
Copy link
Collaborator Author

ip1996 commented Mar 10, 2021

Sorry for the late answer @gerazo, my biggest obstacle is the lack of a machine with Windows OS. I started to work on this when I was in my hometown where there is a Windows PC. Unfortunately, I'm rarely home. That's the reason I haven't answered yet, because currently I don't know the configuration (but I used MinGW-64 with some additional dll). So if anyone in the team is willing to work on this, feel free to modify this branch, but I will try to update this branch too, when I have the tools.

@ip1996
Copy link
Collaborator Author

ip1996 commented Apr 3, 2021

Actually, I'm using GNU (MinGW build tools) instead of MSVC, and the version is GNU 10.2.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants