Support generic chunking and chunk Bind's DOM scan by dreamofabear · Pull Request #6967 · ampproject/amphtml

dreamofabear · 2017-01-10T19:53:06Z

Partial for #6199 and #6910.

Add new PriorityQueue data structure
Extend chunk.js to support non-startup tasks
Chunk Bind's DOM scan

This PR is on the larger side -- happy to split this into multiple PRs. PTAL!

dreamofabear · 2017-01-10T19:55:33Z

src/utils/priority-queue.js

+export default class PriorityQueue {
+  constructor() {
+    /** @private @const {Array<{item: T, priority: number}>} */
+    this.queue_ = [];


We'd get theoretically better performance with a max heap for large N but I thought it was unnecessary for current usage. I'm not sure that even the binary search is worth the readability cost.

dvoytenko · 2017-01-10T22:33:22Z

extensions/amp-bind/0.1/bind-impl.js

    /** @type {!Array<./bind-evaluator.EvaluateeDef>} */
    const evaluatees = [];

+    // TODO(choumx): Use TreeWalker if this is too slow.


I'm pretty sure from past experience that TreeWalker and getElementsByTagName will be generally comparable, but TreeWalker would be somewhat faster.

I actually meant to use TreeWalker so the traversal can be amortized over several frames. In my profiling, this call only took a few ms so I'll work on expression parsing first (which is much slower).

Apparently TreeWalker is really fast.

Cool, thanks for the link.

dvoytenko · 2017-01-10T22:36:02Z

extensions/amp-bind/0.1/bind-impl.js

+
+    // Helper function for scanning a slice of elements.
+    const scanFromTo = (start, end) => {
+      for (let i = start; i < Math.min(end, elements.length); i++) {


Don't remember 100%, but I believe calling elements.length is here would force some significant amount of work. It might be better to iterate until we arrive at elements[++i] === undefined.

Good tip. Looks like length is slow for live collections. Done.

dvoytenko · 2017-01-10T22:38:16Z

src/chunk.js

+   * @return {string}
+   */
+  get name() {
+    return this.fn_.displayName || this.fn_.name;


I think we decided not to use getters like this since they are somewhat slow. /cc @jridgewell .

Also, displayName is good for local debug, but most are likely stripped by compiler for prod binaries.

Probably slower than property access, but what about vs. normal function invocation?

This usage of displayName was ported from original code on line 175.

Getters and setters are considerably slower than method invocation.

Thanks for the reference. Changed to plain method.

dvoytenko · 2017-01-10T22:44:37Z

Thanks, @choumx!

I'd like @cramforce to review in more detail the chunking code changes.

I left few comments, but some of a high-level picture questions I have:

Do we really need priority queue? I understand all startup chunking should be done before amp-binder is instrumented, right?
Is chunking the right instrument here? Or should we look into what Resources manager is doing?
Is there some condition (e.g. user action) where we need to force-execute all pending crawl operations before we proceed with evaluations? Is this something that can be accomplished with chunking/idle callbacks?

cramforce · 2017-01-11T01:11:07Z

extensions/amp-bind/0.1/bind-impl.js

+    };
+
+    // Divide elements into buckets to be scanned, one bucket per chunk.
+    const bucketSize = 10;


I think this is not the right way to do this when you design for requestIdleCallback, but this is a perfect use case for RIC!

The current chunking code has the issue that all tasks that we scheduled could not be split up further (easily). Here you can basically do work until you shouldn't anymore. With that the timeRemaining function should be made available to the scanner and it should always try to do as much as it can given the time available.

Good point! Added support for this by passing the RIC info object into the chunk function. This way, we can still prioritize usage of requestIdleCallback rather than "first-always-wins".

cramforce · 2017-01-11T01:11:56Z

extensions/amp-bind/0.1/bind-impl.js

-          });
+
+    // Helper function for scanning a slice of elements.
+    const scanFromTo = (start, end) => {


How long does a full run take on a typical mobile device?

For a page with 5000 elements with 1000 bindings:

DOM scan: 100ms

Expression parsing: 1s

Complex expressions still take ~1ms each to parse during init. Next step is to either chunk expression parsing or convert BindEvaluator into a web worker.

dreamofabear

Thanks for the review!

Do we really need priority queue? I understand all startup chunking should be done before amp-binder is instrumented, right?

True, though generic background task execution is probably useful to have. Using requestIdleCallback directly has the downside that it's not widely supported and callbacks are executed in order of invocation (which may not be desired).

Is chunking the right instrument here? Or should we look into what Resources manager is doing?

Would you mind elaborating on what you mean by Resources?

Is there some condition (e.g. user action) where we need to force-execute all pending crawl operations before we proceed with evaluations? Is this something that can be accomplished with chunking/idle callbacks?

Good question. It's feasible to implement timeouts with chunk(), but not blocking the UI is the correct UX trade off IMO. Nonetheless it's still important that for vast majority of pages, this completes soon after page render.

dreamofabear · 2017-01-12T01:16:58Z

extensions/amp-bind/0.1/bind-impl.js

+
+    // Helper function for scanning a slice of elements.
+    const scanFromTo = (start, end) => {
+      for (let i = start; i < Math.min(end, elements.length); i++) {


Good tip. Looks like length is slow for live collections. Done.

dreamofabear · 2017-01-12T01:18:26Z

extensions/amp-bind/0.1/bind-impl.js

    /** @type {!Array<./bind-evaluator.EvaluateeDef>} */
    const evaluatees = [];

+    // TODO(choumx): Use TreeWalker if this is too slow.


I actually meant to use TreeWalker so the traversal can be amortized over several frames. In my profiling, this call only took a few ms so I'll work on expression parsing first (which is much slower).

dreamofabear · 2017-01-12T01:25:19Z

src/chunk.js

+   * @return {string}
+   */
+  get name() {
+    return this.fn_.displayName || this.fn_.name;


Probably slower than property access, but what about vs. normal function invocation?

This usage of displayName was ported from original code on line 175.

dreamofabear · 2017-01-12T22:44:50Z

extensions/amp-bind/0.1/bind-impl.js

-          });
+
+    // Helper function for scanning a slice of elements.
+    const scanFromTo = (start, end) => {


For a page with 5000 elements with 1000 bindings:

DOM scan: 100ms

Expression parsing: 1s

Complex expressions still take ~1ms each to parse during init. Next step is to either chunk expression parsing or convert BindEvaluator into a web worker.

dreamofabear · 2017-01-13T23:18:55Z

extensions/amp-bind/0.1/bind-impl.js

+    };
+
+    // Divide elements into buckets to be scanned, one bucket per chunk.
+    const bucketSize = 10;


Good point! Added support for this by passing the RIC info object into the chunk function. This way, we can still prioritize usage of requestIdleCallback rather than "first-always-wins".

dvoytenko · 2017-01-17T18:16:22Z

Would you mind elaborating on what you mean by Resources?

I meant Resources class. It has some knowledge of host "busy" the system is: whether the document is currently being actively scrolled, which elements are being layout and with what priority.

Good question. It's feasible to implement timeouts with chunk(), but not blocking the UI is the correct UX trade off IMO. Nonetheless it's still important that for vast majority of pages, this completes soon after page render.

I think we need to find a paradigm that will work here w/o risk conditions. I see your point and agree that we don't want to block user action. So, instead, I think we need to have a model where we update data structures in memory and mark them as "user action". When a dependent bind expression is ingested, we determine whether it's in initial state (no work) or update by user action (update as soon as practical).

jridgewell · 2017-01-17T23:15:35Z

src/chunk.js

+ * @param {function(?IdleDeadline)} fn
 */
-export function chunk(nodeOrAmpDoc, fn) {
+export function startupChunk(nodeOrAmpDoc, fn) {


I don't understand why startupChunk implies "deactivate-able" while normal chunk is always "active".

Chunking used to only be used for startup tasks, so I guess the nochunking=1 test query was very useful for testing. There's less of a need for generic background task chunks.

Added it below though since it's better to be consistent here.

jridgewell · 2017-01-17T23:19:05Z

src/chunk.js

+    this.viewer_ = null;
+
+    if (viewerOrPromise instanceof Promise) {
+      viewerOrPromise.then(viewer => {


Is the sync case absolutely necessary?

Yes. schedule_() is called synchronously after a task is queued/executed, which calls StartupTask#immediateTriggerCondition_(), whose return value depends on the existence of viewer_.

We can change schedule_() to be executed as a micro-task instead, but I'd rather avoid behavioral changes in this already large-ish PR.

Yes, please. Everything here is very subtle.

Done.

Used existing resolved Promise and added new executeASAP_() to be stubbed in tests instead.

jridgewell · 2017-01-17T23:27:15Z

src/chunk.js

-      t();
+      t.runTask(idleDeadline);
    } catch (e) {
-      // We run early in init. All errors should show the doc.


Catch block can be removed now.

Good call. Done.

dreamofabear · 2017-01-18T20:23:30Z

@dvoytenko

I meant Resources class. It has some knowledge of host "busy" the system is: whether the document is currently being actively scrolled, which elements are being layout and with what priority.

Ah I see, thanks. IMO, requestIdleCallback is the right tool here since it gives us exactly what we want: executing tasks during page idle time. Wrapping RIC with chunk is useful because:

Allows backup functionality when RIC is not supported (Safari and Edge/IE)
Allows prioritization of tasks passed to RIC rather than strict FIFO

Somewhat related to Resources: Malte mentioned it's possible to polyfill requestIdleCallback with RAF, but it's best-effort and may cause battery drain. I think that may be overkill for Bind's use case.

So, instead, I think we need to have a model where we update data structures in memory and mark them as "user action". When a dependent bind expression is ingested, we determine whether it's in initial state (no work) or update by user action (update as soon as practical).

So actually this is only used during Bind's initialization (DOM scan) during page load. This won't affect latency when updating Bind state from user actions.

dvoytenko · 2017-01-18T20:54:02Z

@choumx

So, instead, I think we need to have a model where we update data structures in memory and mark them as "user action". When a dependent bind expression is ingested, we determine whether it's in initial state (no work) or update by user action (update as soon as practical).

So actually this is only used during Bind's initialization (DOM scan) during page load. This won't affect latency when updating Bind state from user actions

Let me clarify this. Imagine the following sequence:

We start expression discovery/parsing. It has a number of bindings and one of the last is bound to selectedIndex var, e.g. [disabled]="selectedIndex == 0". Let's call it button1. However, our discovery/parsing haven't made it that far.
Use scrolls the carousel to the next slide. on-action updates the selectedIndex (on="tap:AMP.setState(selectedIndex=event.slide)").
Binder service executes data change over the so-far-known bindings. However, button1 is not parsed yet, so it's not updated here.
Finally, discovery/parser parses button1. But since it's the "initial" pass, the expression is not evaluated and the button remains disabled since the user action is not recognized.

If my reading of this is correct, we could change this in a couple of ways:

Change step (3) to lock on parsing all expressions. In this we need to have a mechanism to force-complete discovery/parsing.
Or we could change step (4) and stipulate that after the user action, we execute and update expressions since they may have been updated by the user action. Or we could be more precise and mark each variable (selectedIndex in this case) as updated by user action and automatically apply existing expressions.

Does this make sense?

dreamofabear · 2017-01-18T21:42:18Z

@dvoytenko

Or we could change step (4) and stipulate that after the user action, we execute and update expressions since they may have been updated by the user action.

This actually exists in this PR via flag digestQueuedAfterScan_ in bind-impl.js. This reevaluates all expressions rather than just those affected by the user action, but should be fast with the new AST optimization.

dvoytenko · 2017-01-18T21:55:02Z

extensions/amp-bind/0.1/bind-impl.js

+      // Trigger verify-only digest in development.
+      const development = getMode().development;
+      if (development || this.digestQueuedAfterScan_) {
+        this.digest_(/* opt_verifyOnly */ development);


Ok, should not this then be !this.digestQueuedAfterScan_?

Good catch! Thanks.

dvoytenko · 2017-01-18T21:56:32Z

extensions/amp-bind/0.1/bind-impl.js

    if (!opt_skipDigest) {
-      this.digest_();
+      // If scan hasn't completed yet, set `digestQueuedAfterScan_`.
+      if (this.boundElements_) {


Isn't boundElements_ always truthy?

Oops, bad merge I think. Thanks!

dvoytenko · 2017-01-18T21:59:50Z

@choumx

This actually exists in this PR via flag digestQueuedAfterScan_ in bind-impl.js. This reevaluates all expressions rather than just those affected by the user action, but should be fast with the new AST optimization.

I see. This is what I wanted to see. So, in our world, AMP.setState() is always equivalent to the user action, right?

dreamofabear

@dvoytenko

So, in our world, AMP.setState() is always equivalent to the user action, right?

Yes, with the exception of initializing <amp-state> components.

dreamofabear · 2017-01-18T22:03:24Z

extensions/amp-bind/0.1/bind-impl.js

+      // Trigger verify-only digest in development.
+      const development = getMode().development;
+      if (development || this.digestQueuedAfterScan_) {
+        this.digest_(/* opt_verifyOnly */ development);


Good catch! Thanks.

dreamofabear · 2017-01-18T22:08:17Z

extensions/amp-bind/0.1/bind-impl.js

    if (!opt_skipDigest) {
-      this.digest_();
+      // If scan hasn't completed yet, set `digestQueuedAfterScan_`.
+      if (this.boundElements_) {


Oops, bad merge I think. Thanks!

cramforce · 2017-01-18T23:48:27Z

extensions/amp-bind/0.1/bind-impl.js

+
+    // Helper function for scanning a slice of elements.
+    const scanFromTo = (start, end) => {
+      for (let i = start; i < end && elements[i]; i++) {


Optimization: Hoist assignment of elements[i] into loop header to avoid accessing the live-list twice per entry.

cramforce · 2017-01-18T23:50:48Z

extensions/amp-bind/0.1/bind-impl.js

+        if (idleDeadline) {
+          if (idleDeadline.didTimeout) {
+            // On timeout, scan the remaining elements immediately.
+            scanFromTo(position, Number.POSITIVE_INFINITY);


This is OK for now, but not sure the right algorithm. You still want to yield every so often on timeout, I think.

cramforce · 2017-01-18T23:51:27Z

extensions/amp-bind/0.1/bind-impl.js

+        } else {
+          // If `requestIdleCallback` isn't available, scan elements in buckets.
+          // Bucket size is a magic number that fits within a single frame.
+          const bucketSize = 250;


Should this just be what happens on timeout of idle callback?

cramforce · 2017-01-19T00:00:47Z

src/chunk.js

+ * A default chunkable task.
+ */
+class Task {
+  /**


Can this class and all methods be private?

I should state @private is a file-level concept.

cramforce · 2017-01-19T00:02:01Z

src/chunk.js

+    this.viewer_ = null;
+
+    if (viewerOrPromise instanceof Promise) {
+      viewerOrPromise.then(viewer => {


Yes, please. Everything here is very subtle.

cramforce · 2017-01-19T00:02:42Z

src/chunk.js

+ * A task that's run as part of AMP's startup sequence.
+ */
+class StartupTask extends Task {
+  /**


Same question: Can all methods be private?

cramforce · 2017-01-19T00:04:54Z

src/chunk.js

+ * @param {ChunkPriority} priority
+ */
+export function chunk(nodeOrAmpDoc, fn, priority) {
+  if (deactivated) {


Do you actually want this?

Don't need it, but it's better to be consistent if URL contains nochunking=1.

Another option is to change the regex to test for nostartupchunking=1 and remove this.

cramforce · 2017-01-19T22:09:27Z

test/functional/test-chunk.js

-        env.sandbox.stub(resolved, 'then', () => {
-          throw new Error('No calls expected .then');
+        const chunks = chunkInstanceForTesting(env.win.document);
+        env.sandbox.stub(chunks, 'executeASAP_', () => {


Asap_ #styleGuidePolice

D'oh, I keep forgetting about this rule. Done.

jridgewell · 2017-01-23T21:27:39Z

extensions/amp-bind/0.1/bind-impl.js

+
+    // Helper function for scanning a slice of elements.
+    const scanFromTo = (start, end) => {
+      for (let i = start, el = elements[i]; i < end && el; i++) {


el is never updated.

Oops, good catch. Fixed with a break which is more readable IMO.

jridgewell · 2017-01-23T21:28:08Z

extensions/amp-bind/0.1/bind-impl.js

+      for (let i = start, el = elements[i]; i < end && el; i++) {
+        const attrs = el.attributes;
+        const bindings = [];
+        for (let j = 0; attrs[j]; j++) {


j < attrs.length?

Wanted to avoid invoking length which is purportedly slow for live lists. Changed to use for..of instead.

jridgewell · 2017-01-23T21:32:30Z

extensions/amp-bind/0.1/bind-impl.js

+        // If `requestIdleCallback` is available, scan elements until
+        // idle time runs out.
+        if (idleDeadline && !idleDeadline.didTimeout) {
+          while (idleDeadline.timeRemaining() > 1 && elements[position]) {


A lot of this elements[index] check weirdness could go away if you made a function that took the element to scan. Then call it from either this while loop, or the above for loop.

Thought about this. The helper method would need a lot of input params (ugly signature).

I think with the weirdness is reduced with the break and for..of.

Just element, evaluatees and boundElements?

Oh, it's not as bad if extracting the inner loop only; done.

Mutating input params is generally bad for readability but I think it's ok in this case.

jridgewell · 2017-01-24T00:39:32Z

extensions/amp-bind/0.1/bind-impl.js

        const bindings = [];
-        for (const attr of el.attributes) {
+        const attrs = el.attributes;
+        for (let j = 0;; j++) {


What's up with your weird for loops? 😛

You could just check j < attrs.length.

Haha I'm not a fan either. They're in response to: #6967 (comment)

Hard to make everyone happy. :)

Cache the length:

for (let j = 0, l = attributes.length; j < l; j++) { }

Doing an out-of-bounds access is likely slower than anything else in here.

Alright, done.

jridgewell · 2017-01-24T00:41:09Z

extensions/amp-bind/0.1/bind-impl.js

+        // If `requestIdleCallback` is available, scan elements until
+        // idle time runs out.
+        if (idleDeadline && !idleDeadline.didTimeout) {
+          while (idleDeadline.timeRemaining() > 1 && elements[position]) {


Just element, evaluatees and boundElements?

dreamofabear · 2017-01-26T19:28:47Z

Gentle ping.

jridgewell

3 things:

We should follow up with a TreeWalker implementation
This is the second Queue class in the codebase. Time to extract?
This is the second binary search in the codebase. Time to extract?

cramforce

Please hold merge until after canary cut (if it has not happened yet).

dreamofabear · 2017-01-27T18:06:15Z

Thanks all!

@jridgewell Filed #7230 to track your suggestions.

@Private

* Squash chunk-scan commits * fix lint error * rename chunk to startupChunk, pass viewer from Chunks to StartupTask * fix lint and type errors * support timeRemaining in chunk tasks & misc clean up * justin's comments * dima's comments * malte's comments * style and lint fix * fix unit tests * missed a couple @Private declarations * fix scan iteration * remove for..of * more iteration on iteration * move element scan to helper method

@Private

* Squash chunk-scan commits * fix lint error * rename chunk to startupChunk, pass viewer from Chunks to StartupTask * fix lint and type errors * support timeRemaining in chunk tasks & misc clean up * justin's comments * dima's comments * malte's comments * style and lint fix * fix unit tests * missed a couple @Private declarations * fix scan iteration * remove for..of * more iteration on iteration * move element scan to helper method

@Private

* Squash chunk-scan commits * fix lint error * rename chunk to startupChunk, pass viewer from Chunks to StartupTask * fix lint and type errors * support timeRemaining in chunk tasks & misc clean up * justin's comments * dima's comments * malte's comments * style and lint fix * fix unit tests * missed a couple @Private declarations * fix scan iteration * remove for..of * more iteration on iteration * move element scan to helper method

dreamofabear requested review from cramforce and dvoytenko January 10, 2017 19:53

dreamofabear commented Jan 10, 2017

View reviewed changes

dvoytenko suggested changes Jan 10, 2017

View reviewed changes

cramforce requested changes Jan 11, 2017

View reviewed changes

dreamofabear force-pushed the chunk-scan branch from f97fc41 to 3d1a6ec Compare January 13, 2017 23:18

dreamofabear commented Jan 13, 2017

View reviewed changes

dreamofabear force-pushed the chunk-scan branch from 3d1a6ec to e1227c3 Compare January 13, 2017 23:38

jridgewell reviewed Jan 17, 2017

View reviewed changes

dvoytenko reviewed Jan 18, 2017

View reviewed changes

dreamofabear commented Jan 18, 2017

View reviewed changes

cramforce requested changes Jan 19, 2017

View reviewed changes

cramforce reviewed Jan 19, 2017

View reviewed changes

dreamofabear force-pushed the chunk-scan branch 2 times, most recently from 7f866aa to 63a658e Compare January 20, 2017 01:03

William Chou added 8 commits January 20, 2017 16:47

Squash chunk-scan commits

0565e33

fix lint error

40a608a

rename chunk to startupChunk, pass viewer from Chunks to StartupTask

e0859d6

fix lint and type errors

7bdcc05

support timeRemaining in chunk tasks & misc clean up

4aec2fb

justin's comments

db13244

dima's comments

05335d8

malte's comments

198417d

style and lint fix

0a0873b

dreamofabear force-pushed the chunk-scan branch from 0e3624b to d7c08d3 Compare January 20, 2017 21:47

fix unit tests

2e1d712

dreamofabear force-pushed the chunk-scan branch from d7c08d3 to 2e1d712 Compare January 20, 2017 23:48

missed a couple @Private declarations

9590578

jridgewell requested changes Jan 23, 2017

View reviewed changes

William Chou added 2 commits January 23, 2017 17:41

fix scan iteration

b563e49

remove for..of

d13d046

jridgewell reviewed Jan 24, 2017

View reviewed changes

William Chou added 2 commits January 24, 2017 14:34

more iteration on iteration

4a8f4f0

move element scan to helper method

8892c4e

jridgewell approved these changes Jan 26, 2017

View reviewed changes

cramforce approved these changes Jan 27, 2017

View reviewed changes

dvoytenko approved these changes Jan 27, 2017

View reviewed changes

dreamofabear merged commit 38afb0d into ampproject:master Jan 27, 2017

dreamofabear deleted the chunk-scan branch January 27, 2017 18:06

dreamofabear mentioned this pull request Aug 26, 2019

Introduce a proper task abstraction in AMP #24109

Closed

This was referenced Apr 9, 2020

🚨 Error: c.apply is not a function. (In 'c.apply(this,arguments)', 'c.apply' is undefined) rcebulko/amphtml#18

Open

🚨 Error: c.apply is not a function. (In 'c.apply(this,arguments)', 'c.apply' is undefined) rcebulko/amphtml#19

Open

ampprojectbot mentioned this pull request Apr 22, 2020

🚨 Error: Cannot define multiple custom elements with the same tag name #27945

Closed

ampprojectbot mentioned this pull request Jul 6, 2020

🚨 Error: Cumulative Layout Shift not supported #29151

Closed

ampprojectbot mentioned this pull request May 1, 2021

🚨 Error: Cannot read property 'init' of null #34165

Closed

ampprojectbot mentioned this pull request Dec 14, 2021

🚨 Error: Invalid array length ampproject/error-reporting#115

Open

ampprojectbot mentioned this pull request Jan 20, 2022

🚨 Error: Invalid array length ampproject/error-reporting#141

Closed

ampprojectbot mentioned this pull request Mar 8, 2022

🚨 Error: Null promise in layoutCallback ampproject/error-reporting#151

Open

Conversation

dreamofabear commented Jan 10, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dvoytenko commented Jan 10, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dreamofabear left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dvoytenko commented Jan 17, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dreamofabear commented Jan 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dvoytenko commented Jan 18, 2017

Uh oh!

dreamofabear commented Jan 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dreamofabear commented Jan 18, 2017 •

edited

Loading

dreamofabear commented Jan 18, 2017 •

edited

Loading