RFR: 8330077: Allow max number of events to be buffered to be configurable to avoid OVERFLOW_EVENT [v2]

[Fabian Meumertzheim]

On Thu, 11 Apr 2024 21:04:13 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

>> src/java.base/share/classes/sun/nio/fs/AbstractWatchKey.java line 53:
>> 
>>> 51:             intValue = 512;
>>> 52:         }
>>> 53:         MAX_EVENT_LIST_SIZE = intValue;
>> 
>> What if `intValue` turns on to be non-positive for some reason? Should the value also be clamped to some maximum positive value, perhaps determined by some characteristic of the system on which the code is running?
>
> If you are going the system property route, then something like this (not tested) might be in order:
> 
> 
>     private static final int SYSTEM_MAX_EVENT_LIST_SIZE = 100_000; // value TBD
>     private static final int DEFAULT_MAX_EVENT_LIST_SIZE = 512;
> 
>     static final int MAX_EVENT_LIST_SIZE;
>     static {
>         String rawValue = GetPropertyAction.privilegedGetProperty
>             ("jdk.nio.file.maxWatchEvents",
>             String.valueOf(DEFAULT_MAX_EVENT_LIST_SIZE));
>         int intValue;
>         try {
>             intValue = Math.min(Math.max(Math.toIntExact(Long.decode(rawValue)),
>                                          DEFAULT_MAX_EVENT_LIST_SIZE),
>                                 SYSTEM_MAX_EVENT_LIST_SIZE);
>         } catch (NumberFormatException nfe) {
>             intValue = DEFAULT_MAX_EVENT_LIST_SIZE;
>         } catch (ArithmeticException ae) { // rawValue overflowed an int
>             intValue = SYSTEM_MAX_EVENT_LIST_SIZE;
>         }
>         MAX_EVENT_LIST_SIZE = intValue;
>     }

Thanks for the suggestion, the validation was definitely lacking. I pushed a new version that uses `Math.clamp` to clamp to `[512, Integer.MAX_VALUE]` and thus at least cover the lower bound. 

I learned that e.g. NTFS allows up to 2^32 entries per directory (in theory, of course), so I don't see a strong argument in favor of an upper bound motivated by hardware. Do you have a particular bottleneck or problematic use case in mind?

What about the case where the event size reaches `Integer.MAX_SIZE`? An OVERFLOW event could be better than an OOM due to the list size getting too large, what do you think?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18739#discussion_r1561797225