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

Make channel.mark() throw and return offset() #8943

Closed
bradcray opened this issue Mar 23, 2018 · 2 comments · Fixed by #13312
Closed

Make channel.mark() throw and return offset() #8943

bradcray opened this issue Mar 23, 2018 · 2 comments · Fixed by #13312

Comments

@bradcray
Copy link
Member

bradcray commented Mar 23, 2018

At present, the channel.mark() routine returns a syserr. It seems more natural for it to throw an error and to return channel.offset() for the callee to optionally use. With this capability, the new fast revcomp versions could be cleaned up modestly by combining the calls to offset() and mark() into a single call (without breaking backwards compatibility on the current version). While here, we should do the same for the _mark() version.

@bradcray
Copy link
Member Author

bradcray commented Mar 23, 2018

@psahabu: If you're looking for some low-hanging error-handling fruit... @benharsh, @mppf: FYI.

Here's a start on it for the mark() version, though I didn't actually capture the error from qio_channel_mark() to throw it upward.

-inline proc channel.mark():syserr where this.locking == false {
-  return qio_channel_mark(false, _channel_internal);
+ inline proc channel.mark():int(64) throws where this.locking == false {
+  const offset = this.offset();
+  qio_channel_mark(false, _channel_internal);
+  return offset;
 }

@bradcray
Copy link
Member Author

And here's the diff that I'd expect in the fast revcomps:

-      // capture the starting offset
-      const descOffset = input.offset();
-
       // Mark where we start scanning (keep bytes in I/O buffer in input)
-      input.mark();
+      const descOffset = input.mark();

@psahabu psahabu self-assigned this Mar 23, 2018
@daviditen daviditen self-assigned this Jun 18, 2019
@lydia-duncan lydia-duncan added this to the J Sprint 32 milestone Jun 18, 2019
daviditen added a commit that referenced this issue Jun 28, 2019
Make `channel.mark()` return the offset and throw if there is an error

[reviewed by @mppf]

Update channel.mark() and channel._mark() to return the marked offset
and throw if there is an error.

Update test/release/examples/benchmarks/shootout/revcomp-fast.chpl to use
the returned offset instead of calling offset() directly.

Resolves #8943.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants