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

Disable unexpected compilation exceptions caching #40

Merged
merged 5 commits into from
Sep 25, 2024

Conversation

anetaporebska
Copy link
Contributor

Since expression compilation can sometimes throw an unexpected java.lang.NullPointerException: Cannot read the array length because "a" is null due to I/O errors, it would be helpful to have the option to disable such error caching. Otherwise, the cached error is returned every time, paralyzing normal scex usage.

@ddworak
Copy link
Member

ddworak commented Sep 18, 2024

Side questions:

  1. Does it ever make sense to cache errors? I assume it does when they come from actual syntax issues, but maybe we could target those directly then.
  2. Are there other errors than NPEs (maybe I/O exceptions?) that should be filtered out before caching?

@anetaporebska
Copy link
Contributor Author

@ddworak

  1. I assume yes, if something goes wrong every time there is no need to recalculate it multiple times.
  2. Probably also yes but I don't know any other specific case that breaks because of random errors (such as VM I/O errors)

@freddie713
Copy link

Code LGTM, but I would love some unit tests for this new option.

@anetaporebska
Copy link
Contributor Author

I've decided to extend the solution of disabling caching to all unexpected exceptions (not only NPE) that occur during compilation. This should minimize the probability that any other unexpected exception will block the cache infinitely.

@freddie713 please check the code one more time, now it was easier to unit test it, as I no longer had to rely on the filesystem.

@anetaporebska anetaporebska changed the title Optional NPE caching during compilation Disable unexpected compilation exceptions caching Sep 25, 2024
@anetaporebska anetaporebska assigned ddworak and unassigned freddie713 Sep 25, 2024
Copy link
Member

@ddworak ddworak left a comment

Choose a reason for hiding this comment

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

LGTM

@anetaporebska
Copy link
Contributor Author

PR summary:

  • default CachingScexCompiler behavior has changed - it now caches only expected compilation exceptions (and successes ofc) to avoid caching for example a random NPE thrown during a machine I/O error
  • new ScexSettings flag was added - cacheUnexpectedCompilationExceptions - if you rely on such unexpected exceptions you can revert the Scex behavior to the previous one by setting it to true

@anetaporebska anetaporebska merged commit 329bfb5 into master Sep 25, 2024
7 checks passed
@anetaporebska anetaporebska deleted the compilation-npe-caching branch September 25, 2024 06:41
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.

3 participants