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

Rename Date to PosixDate #2424

Closed
wants to merge 15 commits into from
Closed

Rename Date to PosixDate #2424

wants to merge 15 commits into from

Conversation

slayful
Copy link
Contributor

@slayful slayful commented Dec 16, 2017

#1902

  • Date -> PosixDate.
  • If any of the creation arguments is negative set them to zero.
  • Tests for PosixDate.

@slayful
Copy link
Contributor Author

slayful commented Dec 20, 2017

Thanks @mfelsche! Do you happen to know how does the merging process work?

@mfelsche
Copy link
Contributor

someone with commit right presses the merge button.

I approved, so we could just merge now, but as i am quite new to the project i would love to have someone else have a thorough look too.

negative_to_zero(seconds),
negative_to_zero(nanoseconds))

fun negative_to_zero(value: I64): I64 =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method would not be private. To make it private you could start the name with an underscore, like _negative_to_zero. Alternatively, you could define it as a lambda in the constructor body since its only used there:

new create(seconds: I64 = 0, nanoseconds: I64 = 0) =>
  let positive_or_zero = {(n: I64): I64 => if n > 0 then value else 0 end  }
  @ponyint_gmtime[None](this,
     positive_or_zero(seconds),
     positive_or_zero(nanoseconds))

@Theodus Theodus changed the title #1902 Date class allows you to create "invalid" dates Change Date class to PosixDate Dec 20, 2017
@SeanTAllen SeanTAllen added the changelog - changed Automatically add "Changed" CHANGELOG entry on merge label Dec 20, 2017
@SeanTAllen SeanTAllen changed the title Change Date class to PosixDate Rename Date to PosixDate Dec 20, 2017
@SeanTAllen
Copy link
Member

@slayful I made the request change that @Theodus asked for.

If this passes CI (it should) then it can be squashed and merged.

Thanks!

@slayful in the future, if you can put "Closes #XXXX" in your commit comment then the corresponding issue will be closed when we merge.

winksaville and others added 8 commits December 20, 2017 22:55
Add use=llvm_link_static which will link llvm statically with ponyc
Support setting the binary executable name with `--bin-name`.
* Use human-friendly package version numbers in AppVeyor.

Also fixes errorlevel returns in the `make.bat` build script.

* make BinTray version match the appveyor version
)

This change is handled in generated code and doesn't affect the runtime
itself.
Prior to this commit, the runtime would start up a specific number
of scheduler threads (by default the same as the number of physical
cores) on initialization and these scheduler threads would run
actors and send block/unblock messages and steal actors from each
other regardless of how many actors and what the workload of the
program actually was. This usually resulted in wasted cpu cycles
and cache thrashing if there wasn't enough work to keep all
scheduler threads busy.

This commit changes things so that the runtime still starts up
the threads on initialization, but now the threads can suspend
execution if there isn't enough work to do to minimize the work
stealing overhead. The rough outline of how this works is:

* We now have three variables related to number of schedulers;
  `maximum_scheduler_count` (the normal `--ponythreads` option),
  `active_scheduler_count`, and `minimum_scheduler_count`
  (a new `--ponyminthreads` option)
* On startup, we create all possible scheduler threads (up to
  `maximum_scheduler_count`)
* We can never have more than `maximum_scheduler_count` threads
  active at a time
* We can never have less than `minimum_scheduler_count` threads
  active at a time
* Scheduler threads can suspend themselves (i.e. effectively
  pretend as if they don't exist)
* A scheduler thread can only suspend itself if its actor queue
  is empty and it has no actors in it's mute map and it would
  normally send a block message
* Only one scheduler thread can suspend or resume at a time (the
  largest one running or the smallest one suspended respectively)
* We can never skip a scheduler thread and suspend or wake up a
  scheduler thread out of order (i.e. thread 6 is active, but
  thread 5 gets suspended or thread 5 is suspended but thread 6
  gets resumed)
* If there isn't enough work and a scheduler thread would normally
  block and it's the largest active scheduler thread, it suspends
  itself instead
* If there isn't enough work and a scheduler thread would normally
  block and it's not the largest active scheduler thread, it does
  normal scheduler block message sending
* If there's a lot of work to do and at least one actor is muted
  in a scheduler thread, that thread tries to resume a suspended
  scheduler thread (if there are any) every time it is about to
  run an actor. NOTE: This could result in a pathological case
  where only one thread has a muted actor but there is only one
  overloaded actor. In this case the extra scheduler threads will
  keep being woken up and then go back to sleep over and over again.
* The overhead to check if this scheduler thread is a candidate to
  be suspended (`&scheduler[current_active_scheduler_count - 1] ==
  current scheduler address`) is a load and single branch check
* The overhead to check if this scheduler thread is a candidate to
  be suspended (because `&scheduler[current_active_scheduler_count
  - 1] == current scheduler address`) but cannot actually be
  suspended because we're at `maximum_scheduler_count ` is one
  branch (this is in addition to the overhead from the previous
  bullet)
* The overhead to check if there are any scheduler threads to
  resume is a load and single branch check

The implementation of the scheduler suspend/resume is different
depending on the platform.

For Windows, it relies on Event Objects and `WaitForSingleObject`
to suspend threads and `SetEvent` to wake suspended threads.

For Linux environments, by default it relies on signals (specifically,
SIGUSR2) as they are quicker than other mechanisms (pthread
condition variables) (according to stackoverflow at:
https://stackoverflow.com/a/4676069 and
https://stackoverflow.com/a/23945651). It uses `sigwait` to
suspend threads and `pthread_kill` to wake suspended threads. The
signal allotted for this is `SIGUSR2` and so `SIGUSR2` has been
disabled for use in the `signals` package with an error indicating
that it is used by the runtime.

For MacOS, we use pthread condition variables is
also available via a `use=scheduler_scaling_pthreads` argument to
make. This implementation relies on pthread condition variables
and frees `SIGUSR2` so it is available for use in the `signals`
package. It uses `pthread_cond_wait` to suspend threads and
`pthread_cond_signal` to wake suspended threads.

The old behavior of having all scheduler threads active all the
time can be achieved by passing `--ponyminthreads=9999999` as an
argument to a program (because minimum scheduler threads is capped
to never exceed total scheduler threads).

This commit also adds DTRACE probes for thread suspend and thread
resume.

This commit also switches from using `signal` to `sigaction` for
the epoll/kqueue asio signals logic because `sigaction` is more
robust and reliable across platforms
(https://stackoverflow.com/a/232711).
@slayful
Copy link
Contributor Author

slayful commented Dec 20, 2017

I didn't expect changes, so I messed up the merge after I addressed the CR myself (went the lambda approach) so made a nasty interactive rebase. Sorry about that.

The branch should be up to date w/ master now and conflicting changes have been removed. The branch should be as it was in 8a5fd4d.

@slayful
Copy link
Contributor Author

slayful commented Dec 20, 2017

In hindsight shouldn't have panicked and just reverted that.
Here's a clean cherry-picked PR with all the changes.
#2436

Sorry for the commotion.

@SeanTAllen
Copy link
Member

Closing in favor of #2436

@SeanTAllen SeanTAllen closed this Dec 21, 2017
@slayful slayful deleted the GH-1902 branch December 21, 2017 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - changed Automatically add "Changed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants