[dart:io] Adds ProcessInfo.{max,current}Rss. Adds OS::MaxRSS on Fuchsia.

runtime/bin/process_patch.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 @patch
 class _WindowsCodePageDecoder {
   @patch
   static String _decodeBytes(List<int> bytes) native "SystemEncodingToString";
 }
 
@@ skipping 147 matching lines @@
   }
 
   static Stream<ProcessSignal> _watchSignalInternal(ProcessSignal signal) {
     if (_signalControllers[signal._signalNumber] == null) {
       _signalControllers[signal._signalNumber] = new _SignalController(signal);
     }
     return _signalControllers[signal._signalNumber].stream;
   }
 }
 
+@patch
+class ProcessInfo {
+  @patch
+  static int get maxRss {
+    var result = _maxRss();

[Vyacheslav Egorov (Google), 2017/04/18 16:02:04]

This code looks bizarre - why is _maxRss simply not throwing itself?

[zra, 2017/04/18 16:07:17]

Two reasons:

1. Calling Dart_ThrowException and Dart_PropagateError is dangerous and often
leads to memory leaks especially in the dart:io implementation, so I avoid them.

2. An OSError is not an Error and so doesn't receive any special treatment when
set as the return value of a native.

[Vyacheslav Egorov (Google), 2017/04/18 16:21:17]

longjmp and setjmp are dangerous indeed[*] - nevertheless the whole VM is built
on top of them and we can hardly avoid using them. Also both of these are APIs
that we provide to external Dart VM users and extension writers - if we avoid
using them, what does it say about our API quality? 

We should either fix these APIs or avoid them entirely, right now we have a
mixture of styles(**) which is very bad sign:  

$ egrep -e "PropagateError|ThrowException" runtime/bin/process.cc | wc -l
20

(*) if only there was a mechanism that seamlessly integrated with C++ exceptions
and ensured correct destruction of all locally constructed objects when
unwinding the stack... ah, wait, there is such a mechanism.

(**) and style with returning Error is extremely confusing. You spent 30 lines
of code writing something that fits into 4 self-documenting lines!

@patch
class ProcessInfo {
  @patch
  static int get maxRss native "ProcessInfo_MaxRSS";
  @patch
  static int get currentRss native "ProcessInfo_CurrentRSS";
}

[zra, 2017/04/18 16:33:58]

Slava, you make some valid points, but:
1. I don't appreciate your snarky tone.
2. If you don't have resources from your team to devote to code-health in
dart:io, then this discussion won't be productive.

+    if (result is OSError) {
+      throw result;
+    }
+    return result;
+  }
+
+  @patch
+  static int get currentRss {
+    var result = _currentRss();
+    if (result is OSError) {
+      throw result;
+    }
+    return result;
+  }
+
+  static _maxRss() native "ProcessInfo_MaxRSS";
+  static _currentRss() native "ProcessInfo_CurrentRSS";
+}
+
 class _ProcessStartStatus {
   int _errorCode; // Set to OS error code if process start failed.
   String _errorMessage; // Set to OS error message if process start failed.
 }
 
 // The NativeFieldWrapperClass1 can not be used with a mixin, due to missing
 // implicit constructor.
 class _ProcessImplNativeWrapper extends NativeFieldWrapperClass1 {}
 
 class _ProcessImpl extends _ProcessImplNativeWrapper implements Process {
@@ skipping 409 matching lines @@
   var process = new _ProcessImpl(
       executable,
       arguments,
       workingDirectory,
       environment,
       includeParentEnvironment,
       runInShell,
       ProcessStartMode.NORMAL);
   return process._runAndWait(stdoutEncoding, stderrEncoding);
 }