Emul io #10229
Emul io #10229
Conversation
Co-authored-by: Colin Alworth <colin@vertispan.com>
| return byteCount; | ||
| } | ||
|
|
||
| public byte[] readAllBytes() throws IOException { |
There was a problem hiding this comment.
Somehow indent is four spaces in this file (before you started editing it), should be two.
| public static <E> SortedSet<E> emptySortedSet() { | ||
| return new UnmodifiableSortedSet<E>(new TreeSet<>()); | ||
| } | ||
|
|
||
| public static <K, V> SortedMap<K,V> emptySortedMap() { | ||
| return new UnmodifiableSortedMap<>(new TreeMap<>()); | ||
| } | ||
|
|
||
| public static <E> NavigableSet<E> emptyNavigableSet() { | ||
| return new UnmodifiableNavigableSet<E>(new TreeSet<>()); | ||
| } | ||
|
|
||
| public static <K, V> NavigableMap<K,V> emptyNavigableMap() { | ||
| return new UnmodifiableNavigableMap<>(new TreeMap<>()); | ||
| } |
There was a problem hiding this comment.
I'm conflicted about these - granted each new "Empty" costs another hundred bytes or so, but it also gives the compiler a chance to say "this instance here is obviously empty so we can optimize out usages of it", so thats why we have special implementations for EmptyList, etc.
It seems at least that we should be able to share some of these impls to reduce how much code is added. It might be possible even to make the existing EmptyMap implement SortedMap and NavigableMap, and in turn it will have an empty set for its keys that is sorted and navigable?
| } | ||
| pos += chunk; | ||
| if (pos == capacity) { | ||
| capacity = Math.min(len, 2 * capacity); |
There was a problem hiding this comment.
Shouldn't this factor on available() too?
That said, if we're growing the buffer anyway, why not just ask for one that is the size we wanted to begin with?That is, drop the 2048 max, and trust that the smaller of len or available() is the size we're going to want.
There was a problem hiding this comment.
Since available() might be wrong in both directions and the default implementation just returns 0, I added 2048 as a fallback in case available() is underestimating. On the other hand just allocating len bytes would be too much, since we set it to 4 billion if we're reading all bytes. But maybe we can just cheat and instead of growing the buffer just push to the JS array ?
There was a problem hiding this comment.
I mean if the user asks for MAX_VALUE and only gets back 3 billion values and we push into a single array, the app is still going to have a bad time.
The actual max for JS arrays in Chrome is less than 200 million anyway
arr = new Array(200000000);
for (var i = 0; i < 100000000;i++) {
arr[i] = i;
}
results in "Invalid array length". Growing the array from 100 million doesn't work either - roughly, the browser has multiple implementations of array depending on how it thinks you're using it, growing it, and none of them do well once you actually start using real amounts of memory.
Once upon a time I started to rewrite how numeric arrays worked in GWT to use typedarrays, but at the time browsers didn't let us put other properties on them to signal what the real type was, so I abandoned that. That is no longer a limitation, but it would be a breaking change for apps that expect primitive arrays to grow as new data is added to them.
The real contract of available() is "how many bytes can be read without blocking", but GWT can never block anyway - so whatever available() returns is how many you can get max, period. I guess growing could leave us some wiggle room here, but there is still no way for the InputStream to get more data than it currently has buffered? I'm trying to imagine a case where this could matter...
InputStream isn't really a great API for GWT to handle, too much reliance on blocking IO. Between that and max possible sizes for arrays, I think we have to take what we can get until there's a use case that makes this matter?
| } | ||
| int temp = pos; | ||
| pos = this.count - pos < byteCount ? this.count : (int) (pos + byteCount); | ||
| pos = this.count < pos + byteCount ? this.count : (int) (pos + byteCount); |
2 participants
Another partial fix for #10221