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

Add CustomTime time mode #65

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

Lysxia
Copy link

@Lysxia Lysxia commented Feb 23, 2025

This option allows users to supply their own time function. (My use case is to get mutator time from the RTS.)

Addresses #60 minus the sub-issue of preventing breaking changes in the future if this option is not general enough.

@Bodigrim
Copy link
Owner

I like the escape hatch! But given that we are extending TimeMode anyway, shall we add explicit cases for mutator_cpu_ns and/or mutator_elapsed_ns as well?

@Lysxia
Copy link
Author

Lysxia commented Feb 24, 2025

OK. I've added the options as regular values that use CustomTime, because it doesn't seem like there is a use in making them constructors. Or am I missing something? I even added wallTime and cpuTime as lower-cased values for consistency. We could also remove the constructors then, but the breakage seems unnecessary at this point.

@Bodigrim
Copy link
Owner

Making them constructors would allow to specify them via command-line arguments which IMHO is quite useful.

parseValue v = case v of
"cpu" -> Just CpuTime
"wall" -> Just WallTime
_ -> Nothing

@Lysxia
Copy link
Author

Lysxia commented Feb 26, 2025

Can't we do this

 parseValue v = case v of 
   "cpu" -> Just CpuTime 
   "wall" -> Just WallTime 
   "mutcpu" -> Just mutatorCpuTime
   "mutwall" -> Just mutatorWallTime
   _ -> Nothing

?

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.

2 participants