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

One mode #50

Merged
merged 3 commits into from
Jan 20, 2016
Merged

One mode #50

merged 3 commits into from
Jan 20, 2016

Conversation

aantron
Copy link
Owner

@aantron aantron commented Jan 19, 2016

See the commit message in the first commit.

Note that whatever race condition may have existed here: rleonid@dbe8a84#diff-d347ee27660b0e186d9f79a1f8950eb1R44 might not exist anymore, because that code is now only called by at_exit, instead of on every unsafe mark.

Cleans up issues described in #41. Briefly, all the modes were
thread-safe due to the nature of the OCaml runtime, so all have been
replaced by the fastest mode.

As a consequence:
- The BisectThread module has been eliminated.
- The runtime has been greatly simplified to a register function
  (init_with_array) and a dump function called by at_exit.
- The old -mode argument now does nothing, but is kept for
  compatibility. A test was added to ensure it can be used.
- The bisect_ppx.fast package is now a duplicate of plain bisect_ppx.
- The big comment in runtime.mli has been updated to reflect the many
  changes since it was written.
- Several tests are no longer relevant and have been eliminated.

The reference outputs in ppx-integration have not been updated. They
will be updated once we are able to run those tests.

Resolves #41.
@rleonid
Copy link
Collaborator

rleonid commented Jan 19, 2016

Do you mind if I take a day or 2 to look this over? I need to allocate a solid amount of time for this that I don't have at the moment.

@aantron
Copy link
Owner Author

aantron commented Jan 19, 2016

Not at all, take your time.

@rleonid
Copy link
Collaborator

rleonid commented Jan 20, 2016

I just realized I hadn't actually correctly implemented Faster in ppx at all. Rather exposed the mode.

@aantron
Copy link
Owner Author

aantron commented Jan 20, 2016

What do you mean? The code seemed true to its description, which is being like -fast but not thread-safe by not even trying to take locks.

@rleonid
Copy link
Collaborator

rleonid commented Jan 20, 2016

This line should have falseconst for Faster.

@aantron
Copy link
Owner Author

aantron commented Jan 20, 2016

Yeah but IIRC that line only controls whether you get a verbose warning about being unsafe. Locking code wasn't generated in -faster, this line controls that.

@aantron
Copy link
Owner Author

aantron commented Jan 20, 2016

Oh, well we are also talking about different aspects of it. I guess the locking code was left in the startup/shutdown procedures with -faster, but it wasn't emitted for the counts, the latter of which is correct and more consequential IMO. You can see the output of -faster in the reference files in the first commit in this PR.

@rleonid
Copy link
Collaborator

rleonid commented Jan 20, 2016

You're right again, still loading this code into context.

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

2 participants