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

How does the IgnoreError LibraryOption work? #78

Closed
lvh opened this issue Sep 20, 2016 · 6 comments
Closed

How does the IgnoreError LibraryOption work? #78

lvh opened this issue Sep 20, 2016 · 6 comments
Milestone

Comments

@lvh
Copy link

lvh commented Sep 20, 2016

I can't seem to find it anywhere in the code; the only mention is the annotation, but I believe that applies to individual methods (the inverse being SaveError). I thought the goal of LibraryOption was to set the default, however AFAICT IgnoreError isn't used anywhere. This matches my observation that adding LibraryOption/IgnoreError (I have a library that doesn't touch errno) doesn't impact performance at all.

@headius
Copy link
Member

headius commented Sep 21, 2016

It appears you are correct; the library option is never observed. We'll need to add that (or figure out when/why it was removed).

@headius headius added this to the 2.0.10 milestone Sep 21, 2016
@lvh
Copy link
Author

lvh commented Sep 21, 2016

Cool! I've been unable to get any of the IgnoreError performance benefits, but that's probably unrelated to this ticket, so I filed #79.

headius added a commit to headius/jnr-ffi that referenced this issue Sep 21, 2016
* SaveError and IgnoreError library options are now honored. The
  default if neither is specified is to save.
* If both SaveError and IgnoreError options or both annotations
  are used at the same time on the same element (class or method),
  save wins (since it has only a small performance effect if the
  save is unwanted, but serious problems if the save is wanted but
  not performed).
* Only the following cases will ignore error:
  a. Ignore option is specified, save option is not specified,
     and method does not specify either annotation.
  b. Ignore annotation is specified and save annotation is not.
* Centralize logic and document matrix.
* Test all combinations in the matrix.
@headius
Copy link
Member

headius commented Sep 21, 2016

I've pushed a rework of how save/ignore work in #80. Can you test my branch and see if it behaves like you expect here and in #79?

@lvh
Copy link
Author

lvh commented Sep 21, 2016

I'd be happy to, but I'm unsure how, since I can't measure the performance benefit. Do you have any recommendations? I guess I can write a custom version of jnr-posix binding getpid or gettimeofday or somet other thing that should return really fast (enough to see the difference)?

headius added a commit that referenced this issue Sep 22, 2016
Improvements and fixes for errno saving to fix #78.
@headius
Copy link
Member

headius commented Sep 22, 2016

We'll call this fixed, since we are now actually processing both the options and the annotations properly.

#79 remains open to see if we're doing something wrong when ignoring errno.

@lvh
Copy link
Author

lvh commented Sep 22, 2016

Thanks! Sorry; I have some personal stuff going on, so I haven't had time to finish that one up yet. Soon; I promise :)

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

No branches or pull requests

2 participants