SharedArrayBuffer and Atomics tests #839

syg · 2017-01-20T23:36:34Z

This PR adds an initial set of SAB and Atomics tests to test262. Running tests require an agent harness. A harness for SpiderMonkey is provided in harness/agent_spidermonkey.js

Test code is 99% by Lars Hansen.

leobalter · 2017-01-23T14:24:06Z

I need to spend more time to review this, I'll only have some available after 5PM EST today.

For now I can tell most of the copyright lines needs to be fixed to 2017 and, probably, Mozilla Corporation (there are some with 2015/6 and other authors).

I also kind don't like to see the spidermonkey - or any other engine - specific agent file here. Those should be provided by test runners, but I can't and won't block this PR while I don't have a proper working solution to offer as an alternative.

For the agent API, I would like some eyes from @bterlson and @bakkot who are involved with runners as well.

syg · 2017-01-23T20:49:08Z

@leobalter Thanks for the comments. I'll fix the copyright line: tests written by @lars-t-hansen are copyright Mozilla Corporation and the ones written by @anba will remain copyright him.

Do you have any thoughts on how to best deal with providing an agent API for testing from the various implementation shells? This comes down to the more general problem of testing functionality that's only made accessible via some other embedding-specific APIs. Perhaps test262 should only document the expected interface?

bterlson · 2017-01-23T21:39:00Z

I am kind of hoping we could get away with a super/subset of the postMessage API. I was planning to discuss with @domenic and @bakkot in person tomorrow to see what we can get away with.

leobalter · 2017-01-23T22:29:16Z

Perhaps test262 should only document the expected interface?

While I'm +1 with that, I believe it's also healthy to solve this with a plan that can be used properly through different implementations.

The postMessage API would be interesting as well.

leobalter

@syg I left some comments. It looks like a lot but it's fair for a 4400 new lines of code and this is a great work from you so far.

I'm also missing cases we could check for byteConversion values, searching the repo - git grep helps a lot here - for testTypedArrayConversions and byteConversionValues might show some cases where we could clone to assert results using SharedArrayBuffer instead of regular ArrayBuffer objects.

I also want to check the specs for Atomics and see of any other case we could have, for that I would need more time.

leobalter · 2017-01-23T22:33:54Z

test/built-ins/Atomics/add/bad-range.js

+                  ];
+
+for ( let View of views ) {
+    let view = new View(sab);


there's already a harness helper that can be used to loop on different TypedArray constructors, even on specific constructors...

testWithTypedArrayConstructors(fn(View) { let view = new View(sab); ... }, views) can be used as it also tells where the error is if an assertion fail.

leobalter · 2017-01-23T22:35:32Z

test/built-ins/Atomics/add/good-views.js

+    // Result is "negative" though subject to coercion
+    view[3] = -5;
+    control[0] = -5;
+    assert.sameValue(Atomics.add(view, 3, 0), control[0]);


those comments would serve great as assertion messages. Once again, it helps identifying the point of failure.

assert.sameValue(Atomics.add(view, 3, 0), control[0], 'Result is "negative" though subject to coercion');

leobalter · 2017-01-23T22:36:57Z

test/built-ins/Atomics/add/good-views.js

+        // Atomics.store() computes an index from Idx in the same way as other
+        // Atomics operations, not quite like view[Idx].
+        Atomics.store(view, Idx, 37);
+        assert.sameValue(Atomics.add(view, Idx, 0), 37);


once again an assertion message here indicating the used index

leobalter · 2017-01-23T22:39:00Z

test/built-ins/Atomics/add/length.js

@@ -0,0 +1,28 @@
+// Copyright (C) 2015 André Bargull. All rights reserved.


This test format is based from a work of André Bargull, but the test for this specific case (length prop of Atomics.add is made by you, so Mozilla Corporation applies well here, AFAIK.

leobalter · 2017-01-23T22:42:22Z

test/built-ins/Atomics/add/non-views.js

+              new Map(),
+              new Set(),
+              new WeakMap(),
+              new WeakSet(),


while it does not hurt adding more and more, I don't believe it's really necessary to have extra objects as instances of Map, Set, Date, Error, etc...

A case with all the primitive values, an array, an ordinary object, should be enough.

leobalter · 2017-01-23T22:45:25Z

test/built-ins/Atomics/compareExchange/bad-range.js

+                    (view) => { password: "qumquat" },
+                    (view) => ({ valueOf: () => 125 }),
+                    (view) => ({ toString: () => '125', valueOf: false }) // non-callable valueOf triggers invocation of toString
+                  ];


bad_indices is reused many times, we should store it in a harness file to be included in each of these cases.

same for good_indices

leobalter · 2017-01-23T22:50:42Z

test/built-ins/TypedArrays/buffer-arg-returns-new-instance.js

 testWithTypedArrayConstructors(function(TA) {
  var bpe = TA.BYTES_PER_ELEMENT;

-  var buffer1 = new ArrayBuffer(bpe * 4);
+  var buffer1 = new Buffer(bpe * 4);


While I appreciate this makes the test case shorter, it's appropriate to split these cases in two different files, one using regular ArrayBuffer and another one using SharedArrayBuffer, including a frontmatter features tag for it, this way this test won't start failing in other implementations without this new feature, but still passing for cases w/ ArrayBuffer.

this applies for every similar change in this branch, including those on the DataView folder.

for ( let Buffer of [ArrayBuffer, SharedArrayBuffer] ) { will help finding most.

leobalter · 2017-01-23T22:52:25Z

test/built-ins/SharedArrayBuffer/prototype/slice/tointeger-conversion-start.js

+  The `start` index parameter is converted to an integral numeric value.
+info: >
+  SharedArrayBuffer.prototype.slice ( start, end )
+


extra line, can be removed

leobalter · 2017-01-23T22:54:01Z

test/built-ins/SharedArrayBuffer/prototype/slice/tointeger-conversion-start.js

+var result = arrayBuffer.slice(start, end);
+assert.sameValue(result.byteLength, 4, "slice(4.5, 8)");
+
+var start = NaN, end = 8;


redeclaring values.

Also, start and end values can be set directly to .slice and result should be declared only once.

same applies for

test/built-ins/SharedArrayBuffer/prototype/slice/tointeger-conversion-end.js

test/built-ins/SharedArrayBuffer/prototype/slice/start-exceeds-length.js

leobalter · 2017-01-23T22:58:13Z

test/built-ins/SharedArrayBuffer/prototype/byteLength/this-is-not-object.js

+// This code is governed by the BSD license found in the LICENSE file.
+/*---
+description: Throws a TypeError exception when `this` is not Object
+features: [Symbol]


at this point I don't think Symbol should be flagged as a feature anymore, this is a test for ES2017 and Symbol is from 2 years ago. We can get rid of these on this branch.

Same for DataView and TypedArrays in other files.

syg · 2017-01-23T23:13:23Z

@leobalter Thanks for the super speedy review! Will address by end of the day.

leobalter · 2017-01-23T23:15:17Z

I'm on the East Coast and currently using my free time, so my next availability will be tomorrow after 5PM EST as well. I'm glad to help

syg · 2017-01-24T03:09:32Z

@leobalter Here are the changes addressing your comments:

Refactored all of Atomics/ to have testWithTypedArrayConstructors-style functions for the bad indices, the good indices, and the non-view values.
Split out SAB tests in TypedArray/ and DataView/ to their own files, with the -sab suffix. The old tests are reverted to testing ArrayBuffer only. These tests all have the features: [SharedArrayBuffer] preamble.
Removed features: [Symbol] and other ES6-era features from the split-out tests
Changed most of the copyright lines to say Mozilla.

lars-t-hansen · 2017-01-24T07:42:02Z

@syg, IANAL, but note that many tests were cloned from existing tests in the repo (ArrayBuffer, TypedArray, DataView) and that changing the copyright lines without considering that may violate the original copyright.

@bterlson

I am kind of hoping we could get away with a super/subset of the postMessage API.

That requires a concrete proposal from you for what the shell API should be. Might I also point out that thus far, Microsoft reps pinged here and on email have been completely silent about what you are willing to implement (even more so than Apple :-) I'm happy to discuss & implement concrete proposals but not to keep throwing proposals out there in the hope of some feedback.

Signals I've received so far suggest that the current proposal in the test262 PR is implementable in the Chrome shell and that Apple would be OK implementing the SpiderMonkey API in the shell, allowing the proposal in the PR to work in the webkit shell as well.

leobalter · 2017-01-24T15:27:21Z

@syg, IANAL, but note that many tests were cloned from existing tests in the repo (ArrayBuffer, TypedArray, DataView) and that changing the copyright lines without considering that may violate the original copyright.

@lars-t-hansen I understand the concern and that's also why I mentioned this is what we do conventionally.

@bterlson what is the best strategy here?

Signals I've received so far suggest that the current proposal in the test262 PR is implementable in the Chrome shell and that Apple would be OK implementing the SpiderMonkey API in the shell, allowing the proposal in the PR to work in the webkit shell as well.

In this case it doesn't need to be flagged as SM specific, right? Just an api, I guess.

lars-t-hansen · 2017-01-24T15:43:01Z

@leobalter, the implementation of $.agent embedded in the PR works with the SpiderMonkey API; the v8 shell has a different API (more like a browser) and would need a different implementation. @binji might know more.

(Thanks for vetting this BTW. As for comprehensiveness, it's not quite there, but I think all the major points are touched upon.)

leobalter · 2017-01-24T16:08:49Z

that goes back to the original problem, whether this should be implemented in this project or being project/runner specific. test262 relies on generic implementations, runtime specific parts is out of its own maintenance scope.

lars-t-hansen · 2017-01-24T16:11:23Z

Absolutely. I guess if we want this in the project then we either agree on an API or we do some sniffing in the harness; I'm fine either way. The file agent_spidermonkey.js is only meant as an example implementation (in my own fork), it was not my intent to land this in the test262 repo.

leobalter · 2017-01-24T16:39:28Z

Thanks!

I have one idea and I need you @lars-t-hansen and @syg to agree. We merge this without the file but leave a reference link in the api doc for a gist (or something else) with the contents of agent_spidermonkey.js as an example.

If it works for you, I can do that.

The feedback changes for the review is great so far. I only want to do a final check for tests coverage tonight. This will take a couple hours comparing what we have with the current specs.

In a high level perspective it seems fine so far.

lars-t-hansen · 2017-01-24T16:44:47Z

@leobalter, it works for me.

…called on values of each other

binji · 2017-01-25T01:15:00Z

@lars-t-hansen Yes, the d8 API is very similar to the browser API, with the one difference that when receiving you block when receiving a message from the worker. This is because there is no event loop on the main thread in d8.

I still haven't had a chance to implement the d8 equivalent of $.agent, but I believe with Lars's changes to receiveBroadcast (so it takes a callback instead of blocking) it should work.

…d TypedArrays

syg · 2017-01-25T07:07:15Z

@anba I attempted to address all your comments. I did not make an SAB variant of TypedArray/prototype/set/typedarray-arg-set-values-same-buffer-other-type.js because I did not understand what was going on in that test.

bterlson · 2017-01-25T19:47:31Z

@lars-t-hansen sorry I missed your previous post - sorry for silence. I will of course make a proposal (hopefully soon), but I think it's isomorphic with your current approach (with the exception of broadcast being hard to implement in browsers, I think) so shimming should be possible so at least eshost could expose a more familiar API.

anba · 2017-01-26T12:24:17Z

@anba I attempted to address all your comments.

👍

I did not make an SAB variant of TypedArray/prototype/set/typedarray-arg-set-values-same-buffer-other-type.js because I did not understand what was going on in that test.

I think this file wants to test the following conditions:

%TypedArray%.prototype.set is called on two typed arrays which share the same underlying ArrayBuffer.
The element type of the typed arrays is different.
The chosen offsets ensure to have overlapping ranges in the ArrayBuffer, so the effect of CloneArrayBuffer is visible. That means an implementation which skips CloneArrayBuffer will not pass the test.

binji · 2017-01-27T22:30:00Z

FYI: I was able to write an agent implementation for d8, and run it over the tests, works well (minus a few corner cases I forgot to implement 😄).

littledan · 2017-01-28T03:10:54Z

Would it be reasonable to put a feature tag or required js file on these tests (or whatever else would make sense in the YAML metadata)? That would make it possible to make the agent extensions only for this set of tests. Something like that is done for things which need to detach an ArrayBuffer. Given the global state required for the agent, it would be nice to not have to instantiate it if it's not used.

syg · 2017-01-28T03:25:37Z

@littledan which tests? SharedArrayBuffer tests outside of the SharedArrayBuffer directory should have `features: [SharedArrayBuffer]`. Are you suggesting something like that for agent code? I'm not sure what's best there.

…

On Fri, Jan 27, 2017 at 7:10 PM, littledan ***@***.***> wrote: Would it be reasonable to put a feature tag or required js file on these tests (or whatever else would make sense in the YAML metadata)? That would make it possible to make the agent extensions only for this set of tests. Something like that is done for things which need to detach an ArrayBuffer. Given the global state required for the agent, it would be nice to not have to instantiate it if it's not used. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#839 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAF10ete5p6cRkVHCc3ukp3j3o0nHaZIks5rWrG_gaJpZM4Lp3NW> .

-- shu

littledan · 2017-01-28T03:29:24Z

OK, my mistake, maybe the information is already there, if the logic is, "agent code may be required from built-ins/Atomics, built-ins/SharedArrayBuffer, or something with the [SharedArrayBuffer] feature tag", and no changes are needed. Would this be accurate?

syg · 2017-01-28T03:32:42Z

@littledan I don't know how the `features` tag currently works. I thought it was a simple test for the extistence of the global property listed, I'm not sure if it's able to interpret them arbitrarily, like requiring there being an embedding-provided implementation of agent code. Perhaps @leobalter knows?

…

On Fri, Jan 27, 2017 at 7:29 PM, littledan ***@***.***> wrote: OK, my mistake, maybe the information is already there, if the logic is, "agent code may be required from built-ins/Atomics, built-ins/SharedArrayBuffer, or something with the [SharedArrayBuffer] feature tag", and no changes are needed. Would this be accurate? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#839 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAF10Wa2nn3F7JWUPyjm1jYaHNVT5zfKks5rWrYVgaJpZM4Lp3NW> .

-- shu

littledan · 2017-01-28T03:34:20Z

Well, it's just metadata in YAML, the user of the test can do whatever they want with it (right?). My idea was to put something hanging off of it in V8's test infrastructure. When I wrote, "if the logic is" above, I meant for that logic to (possibly) be encoded in the V8 test262 infrastructure.

So my question would be, is it true that everything using the agent API will be in one of those three states? If not, maybe we could mark tests using it somehow (besides searching for $.agent in the source).

syg · 2017-01-31T08:21:21Z

@leobalter I'm not sure how to mark your requested changes as addressed. In case it wasn't clear, I'm done with responding to review from my end.

syg · 2017-01-31T08:26:26Z

@littledan Right now agents code is only required in built-in/Atomics tests. The intended logic for features: [SharedArrayBuffer] is simply that the SharedArrayBuffer constructor exists on the global. I see no problems with making it also imply Atomics and agents are available.

OTOH I've seen more fine-grained features: tags like features: [Reflect.construct] vs just features: [Reflect]. I'm not sure what the advantage of being more fine-grained in the SAB case is. Atomics and agents are not useful without SAB and vice versa.

lars-t-hansen · 2017-01-31T08:32:05Z

@syg, in principle SAB is useful w/o Atomics, you can synchronize by other means (eg postMessage).

Other features to consider that are more subtle:

whether Atomics.wait can wait on the main thread
timer resolution for timeouts

By and large I would say it's not worth worrying about these, but they are open aspects of the spec that one could in principle test. For example, if the embedding claims that Atomics.wait can/cannot wait on the main thread, then one can test that.

Ditto endianness. One presumes the DataView and TypedArray specs already deal with that in some way, and test it.

bakkot · 2017-01-31T19:34:03Z

Misc thoughts on the API to follow. Sorry for the delay.

@littledan: My plan was to conditionally define the agent API depending on if the test contains the string $.agent, but I agree that's not ideal.

The features tag isn't actually referred to in INTERPRETING.md, so I'm not sure what all is appropriate for it; I don't think it's used very consistently. That said, it isn't just a check for global properties: e.g.

Thoughts on including agent in the features list for tests which need to make use of an agent? Ping @jugglinmike.

I'm a little concerned about $.sleep in browser environments. I can make it work, sort of, by parsing and rewriting tests which call it to make them asynchronous (not actually that hard), but... thoughts on making those tests either async-await or callback-based?

getReport should probably be defined in a harness file instead of being copied 14 times. I'm also a little concerned about it waiting forever if no message is posted: perhaps we could include something in INTERPRETING.md similar to the async tests, to the effect of 'The implementation is free to select an appropriate length of time to wait before considering the test "timed out" and failing.'

Also, if the callees of sleep are rewritten to be asynchronous, getReport could maybe be promise-based.

The text for $.agent.broadcast says it is a "a function that takes a SharedArrayBuffer and an Int32", but it looks like it's only ever called with just the buffer. Also, half of the callees of receiveBroadcast pass a function of two arguments (sab and id) which then never refers to the second.

I'd prefer the various wait times be host-definable, I think (e.g. this one)- possibly as properties of $.agent? A host being very slow shouldn't make it impossible to pass these tests.

Alternatively, $ATOMICS_MAX_TIME_EPSILON could be a property of $.agent, and other times could be defined by reference to this (e.g. $.agent.sleep(5 * $.agent.TIME_EPSILON)).

I'm not firmly committed to this - I don't think it'll matter most of the time - but I think it's more in line with the rest of the harness.

lars-t-hansen · 2017-01-31T20:09:44Z

@bakkot, the $.agent framework and many of the tests that are currently written for it are squarely aimed at the JS shells. For a browser we'd restructure these. We can try to factor the commonalities, but since the shells have such different facilities for agents and communication it's not necessarily easy. I agree $.agent.sleep is not good for the browser, but then it was not intended to be.

Most instances of sleep are necessary, given how the tests are written (and the facilities otherwise available in the shells - no timeout callbacks, to my knowledge). If sleep is not possible on the main thread (in a browser) then the tests should probably be rewritten so that two workers implement the tests; surely the workers can sleep. Should probably work in the shells; adds some complexity.

Agreed that the id argument for broadcast is probably unnecessary since the broadcast is synchronous and all agents must participate and receive the broadcasts in the same order. It made more sense when the broadcasts were asynchronous.

I'd like to push back against parameterizing on the sleep times until it is shown to be a problem in practice, it just makes the tests harder to understand. I agree that it can be a problem but I'm not convinced it will be a problem.

bakkot · 2017-01-31T20:37:32Z

@lars-t-hansen, I intend to get these working on browsers, like the rest of test262, regardless of what they're written to target. It's just a little harder if they use sleep in synchronous code. I do think browsers should be a first-class consumer of test262, though.

no timeout callbacks, to my knowledge

Given sleep(), it's trivial to write one, surely? function timeout(fn, ms) { sleep(ms); fn(); }

parameterizing on the sleep times [...] just makes the tests harder to understand

Not sure I agree, but I'm fine with leaving it as-is if you feel strongly.

lars-t-hansen · 2017-01-31T21:51:56Z

parameterizing on the sleep times [...] just makes the tests harder to understand

Not sure I agree, but I'm fine with leaving it as-is if you feel strongly.

Not really, I don't think I should own the decisions made here, just voicing an opinion.

binji · 2017-01-31T22:13:57Z

Well, you could always write sleep like this:

function sleep(ms) {
  var end = +new Date() + ms;
  while (+new Date() < end);
}

I'm assuming it isn't necessary to return to the event loop for any of these tests.

lars-t-hansen · 2017-02-01T07:24:04Z

@binji Interesting point about the event loop. None of the test require an event loop and in principle a sleep as you describe it should work. However, in Firefox the main thread performs work on behalf of other threads from time to time and a blocking sleep as you suggest would get in the way of that. In particular, after creating a new Worker, the script must return to its event loop before the worker becomes active, so code that does

$.agent.start(...)
$.agent.sleep(1000)

would not actually see any communication from the agent after the sleep, no matter how fast the computer, because the new agent has not had a chance to start running. (Again, in the browser.)

Perhaps that suggests more CPS as for broadcast: $.agent.start(..., function () ... ) where the thunk is invoked once the agent is known to be running on its own thread.

Now that I think about it perhaps sleep should be handled that way too. It would be fine for the shell tests; for the browser tests they would use setTimeout.

(EDIT: And of course the observation about sleep is just the observation made by @bakkot above.)

binji · 2017-02-01T20:01:48Z

@lars-t-hansen good point, I'm not certain but I wouldn't be surprised if Blink had the same restriction w.r.t. needing to return to the event loop to spawn a worker.

leobalter · 2017-02-07T16:17:09Z

While the tests are fine, this work needs some discuss on the used api for the test harness.

I believe this can be done in a follow up PR, improving the way we are testing SABs.

Lars T Hansen and others added 2 commits January 20, 2017 15:32

Add SharedArrayBuffer and Atomics tests

b07a01e

Add Atomics[@@toStringTag] test

d1fccaa

This was referenced Jan 20, 2017

test262 style conformance, etc tc39/proposal-ecmascript-sharedmem#159

Closed

Implement SharedArrayBuffer & Atomics tc39/ecma262#769

Closed

leobalter suggested changes Jan 23, 2017

View reviewed changes

leobalter added the awaiting author label Jan 23, 2017

syg added 8 commits January 23, 2017 15:36

Fix new SAB tests' copyright lines to say Mozilla

40bb0d2

Refactor Atomics tests

6e2b2e5

Fix style in SharedArrayBuffer.prototype.slice tests

9d97d3c

Remove features: [Symbol] from SharedArrayBuffer tests

49ab6c5

Split out SharedArrayBuffer versions of TypedArrays/buffer-arg-* tests

291563d

Revert buffer-arg tests to only test ArrayBuffer

388466d

Split out SharedArrayBuffer DataView tests

ab2ca76

Revert DataView tests to only test ArrayBuffer

1297867

leobalter self-assigned this Jan 24, 2017

leobalter removed the awaiting author label Jan 24, 2017

syg added 3 commits January 24, 2017 14:28

Refactor 100 to $ATOMICS_MAX_TIME_EPSILON in atomicsHelper.js

2d74325

Combine two features: lines

28abb1d

Add tests for [Shared]ArrayBuffer.prototype.{byteLength,slice} being …

adae9e8

…called on values of each other

syg added 2 commits January 24, 2017 23:05

Add test for new %TypedArray%(new %TypedArray%(SAB))

70e2166

Add versions of %TypedArray%.prototype.set on SharedArrayBuffer-backe…

f076541

…d TypedArrays

leobalter approved these changes Jan 31, 2017

View reviewed changes

leobalter merged commit a72ee6d into tc39:master Feb 7, 2017

bakkot mentioned this pull request Feb 14, 2017

Support SharedArrayBuffer / Atomics API bakkot/test262-web-runner#3

Open

bterlson mentioned this pull request Mar 20, 2017

Enable SharedArrayBuffer test262 tests in ch.exe chakra-core/ChakraCore#2714

Closed

		@@ -0,0 +1,28 @@
		// Copyright (C) 2015 André Bargull. All rights reserved.

SharedArrayBuffer and Atomics tests #839

SharedArrayBuffer and Atomics tests #839

Conversation

syg commented Jan 20, 2017

leobalter commented Jan 23, 2017

syg commented Jan 23, 2017

bterlson commented Jan 23, 2017

leobalter commented Jan 23, 2017

leobalter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

syg commented Jan 23, 2017

leobalter commented Jan 23, 2017

syg commented Jan 24, 2017

lars-t-hansen commented Jan 24, 2017

leobalter commented Jan 24, 2017

lars-t-hansen commented Jan 24, 2017

leobalter commented Jan 24, 2017

lars-t-hansen commented Jan 24, 2017

leobalter commented Jan 24, 2017

lars-t-hansen commented Jan 24, 2017

binji commented Jan 25, 2017

syg commented Jan 25, 2017

bterlson commented Jan 25, 2017

anba commented Jan 26, 2017

binji commented Jan 27, 2017

littledan commented Jan 28, 2017

syg commented Jan 28, 2017 via email

littledan commented Jan 28, 2017

syg commented Jan 28, 2017 via email

littledan commented Jan 28, 2017 • edited Loading

syg commented Jan 31, 2017

syg commented Jan 31, 2017 • edited Loading

lars-t-hansen commented Jan 31, 2017

bakkot commented Jan 31, 2017 • edited Loading

lars-t-hansen commented Jan 31, 2017

bakkot commented Jan 31, 2017

lars-t-hansen commented Jan 31, 2017

binji commented Jan 31, 2017

lars-t-hansen commented Feb 1, 2017 • edited Loading

binji commented Feb 1, 2017

leobalter commented Feb 7, 2017

littledan commented Jan 28, 2017 •

edited

Loading

syg commented Jan 31, 2017 •

edited

Loading

bakkot commented Jan 31, 2017 •

edited

Loading

lars-t-hansen commented Feb 1, 2017 •

edited

Loading