Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEAT: high res timestamps on W8+ #5295

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

hiiamboris
Copy link
Collaborator

@hiiamboris hiiamboris commented Mar 8, 2023

Example output of probe new-line/all loop 100 [append [] pick now/precise 'second] on:
7.686709000001429 
7.68671650000033 
7.686720099998638 
7.686723400001938 
7.6867265999972005 
7.686729699998978 
7.686732800000755 
7.686736000003293 
7.686738900003547 
7.686741999998048 
7.686744999999064 
7.68674800000008 
7.686751000001095 
7.686753900001349 
7.6867568000016036 
7.686759999996866 
7.686763099998643 
7.686766300001182 
7.686769300002197 
7.686772200002451 
7.6867752999969525 
7.686778199997207 
7.686781199998222 
7.686784299999999 
7.686787300001015 
7.686790400002792 
7.686793399996532 
7.686796399997547 
7.686799299997801 
7.686802299998817 
7.686805199999071 
7.68680840000161 
7.686811400002625 
7.6868153999967035 
7.686818399997719 
7.686821299997973 
7.686824299998989 
7.686827199999243 
7.68683030000102 
7.686833400002797 
7.686836399996537 
7.686839599999075 
7.686842600000091 
7.6868456000011065 
7.686848600002122 
7.686851500002376 
7.68685440000263 
7.6868574999971315 
7.686860599998909 
7.686863599999924 
7.686866700001701 
7.686869800003478 
7.686872799997218 
7.686875899998995 
7.686878799999249 
7.686882000001788 
7.686885100003565 
7.686888099997304 
7.686891199999081 
7.686894099999336 
7.686897100000351 
7.686900000000605 
7.6869029000008595 
7.686906100003398 
7.686909099997138 
7.686912700002722 
7.686915999998746 
7.686918999999762 
7.686922000000777 
7.686925100002554 
7.686928000002808 
7.686931199998071 
7.686934500001371 
7.6869376999966335 
7.686940999999933 
7.686944000000949 
7.686947000001965 
7.686949900002219 
7.686952900003234 
7.686956099998497 
7.686959200000274 
7.686962300002051 
7.686965300003067 
7.686968399997568 
7.686977400000615 
7.68698040000163 
7.686983599996893 
7.68698669999867 
7.686989699999685 
7.686992900002224 
7.686995800002478 
7.686998700002732 
7.687001799997233 
7.687004799998249 
7.687008000000787 
7.6870111000025645 
7.687014299997827 
7.687017299998843 
7.687020299999858 
7.687023200000112

@greggirwin
Copy link
Contributor

Very nice @hiiamboris. So we now have ~0:0:0.000003 resolution.

@hiiamboris
Copy link
Collaborator Author

hiiamboris commented Mar 8, 2023

About 100ns really (precision limit of this API call). But now/precise itself takes ~1us, and now/utc/precise ~0.5us.

On Linux resolution is 1us, now/utc/precise takes 1us, now/precise 6-7us.

@greggirwin
Copy link
Contributor

Dynamic loading may have been a carryover from when @qtxie suggested it for Win7 compatibility.

@dockimbel
Copy link
Member

dockimbel commented Mar 8, 2023

I would have preferred the use of static linking for such import and conditional preprocessing directive to compile the appropriate code depending on the platform. We could have created a new Window7 target for that, using the legacy property that is already there for that (see the WindowsXP target definition).

Also this kind of dynamic linking of kernel libs might increase the false flagging on AVs...

@greggirwin
Copy link
Contributor

Since Win7 isn't supported anymore, do we even need that legacy support? Time marches on.

@dockimbel
Copy link
Member

dockimbel commented Mar 8, 2023

I'm all for dropping entirely Win7 and even Win8 support too. Windows 12 release is next month...

From ghacks:

According to Statcounter's latest information, Windows 7 usage nearly halved in February 2023. The operating system's market share fell from 9.55% in January 2023 to 5.34% in February 2023. Similarly, but at a smaller scale, Windows 8.1's market share dropped from 2.28% in January 2023 to 1.14% in February 2023.

@hiiamboris
Copy link
Collaborator Author

conditional preprocessing directive to compile the appropriate code depending on the platform

But we do not (and should not IMO) offer prebuilt consoles for different Windows versions.

Anyway, as I know a lot of people are still using WinXP, and I myself would want continue using W7 on another PC, and I would find it strange for Red to not run on an OS capable of running any modern 3D game, so I prefer this solution. 2-3 lines of code is not even a price for compatibility.

@hiiamboris
Copy link
Collaborator Author

hiiamboris commented Mar 8, 2023

BTW another reason I did not use the #if directive for W8+ is because I don't fully trust MS when they say it's supported on W8. Maybe on some old build of W8 it's not supported?

@greggirwin
Copy link
Contributor

It's not just the 2-3 lines of code, but having systems we can test on, and dealing with the legacy systems and people that will have problems. I still have Win7 on more than one machine, and a couple old VMs going back to W2K (and DOSBox :^). Always tough choices, and I hold on to old stuff more than many, but we also have to shed baggage if we want to keep moving. The question is what value something adds, versus what it costs to support it.

@greggirwin
Copy link
Contributor

This would all be easier if new systems and OSs were unequivocally better than their predecessors. :^\

@greggirwin
Copy link
Contributor

Also how we make things fail gracefully, so at least something runs, even if not full featured.

@greggirwin
Copy link
Contributor

As far as prebuilt consoles, I agree that we should not keep building old ones. What we can, and should do is make old ones available in a History Archive.

@hiiamboris
Copy link
Collaborator Author

This would all be easier if new systems and OSs were unequivocally better than their predecessors. :^\

Indeed. Unfortunately I expect they will be significantly worse in the next decade or two as they were degrading in the last decade but faster, up to total absurdity :)

@dockimbel
Copy link
Member

dockimbel commented Mar 9, 2023

2-3 lines of code is not even a price for compatibility.

It's not about code size but introducing an exception in the way we handle OS imports for no good reason. Having a specific compilation target for Win7 is perfectly fine, I don't see any reason why not to comply with our existing design choices. Using dynamic loading suddenly "just because you prefer it" is just making the code architecture messier as we introduce an exception.

There is no "compatibility" issue as Win7 users could compile for their own target as WinXP users do so far. We can add some info about that in the README page.

@hiiamboris
Copy link
Collaborator Author

here is no "compatibility" issue as Win7 users could compile for their own target as WinXP users do so far.

I have to strongly disagree. What you're proposing will not only force Win7 users to build their own console, but also will make all compiled Red Windows executables Win7-incompatible. You are proposing that everyone who ever releases an exe have to provide different binaries for WXP, W7 and W8+. Have you ever seen anyone doing that? I haven't, because it would be quite a mess. The worst I've seen is separate 32- and 64-bit binaries, but that's because there's no way around it.

And all that over what? More digits in the timestamp? :)

You cite statcounter, but behind every number there's someone's huge money. Based on my own observations I would not easily bet what's more popular in my country - W10 or WXP . And it's not one of the overexploited countries (yet).

It's understandable that commercial OS vendors would do all they can to push their users into faster and faster cycle of OS upgrades, to lock and spy on their users as much as they can. They just can't do otherwise. But it's always more pain for the users in the end. As developers the least we can do is be a bit more conscious and not help proliferate this madness ourselves. I understand it when maintaining compatibility requires enormous effort, but at least we should not make the effort to break compatibility when we have it almost for free. The ultimate programmers dream should be a single binary that runs on every platform, not a separate binary per every version of every platform.

We have currently another 6 dynamic imports in runtime. One more doesn't make a difference. I have no reason to believe dynamic imports affect AV results in any way. I see no proof of it. VirusTotal shows 7 alerts for the View-less console without any dynamic imports, vs only 4 alerts for the View console that has 7 dynamic imports.

Besides, we have to plan for Red-level runtime library loading, which will unavoidably use the same mechanism.

If after reading my arguments you still can't accept this PR, I would prefer we just close it, rather than add a W8+ required import.

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.

None yet

3 participants