util: refactor format method. #12407

jseijas · 2017-04-14T00:57:40Z

Method format refactored to make it more maintenable, replacing the
switch by a function factory, that returns the appropiated function
given the character (d, i , f, j, s).

Also, performance when formatting an string that contains several
consecutive % symbols is improved. The test:

const numSamples = 10000000;
const strPercents = '%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%s%%%%%%%%%%%%%%%%%i%%%%%%%%%%%%%%%%%%%%%%%%%%';
var s;
console.time('Percents');
for (let i = 0; i < numSamples; i++) {
  s = util.format(strPercents, 'test', 12);
}
console.timeEnd('Percents');

Original time: 28399.708ms
After refactor: 23763.788ms
Improved: 16%

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

Affected core subsystem(s)

Method format refactored to make it more maintenable, replacing the switch by a function factory, that returns the appropiated function given the character (d, i , f, j, s). Also, performance when formatting an string that contains several consecutive % symbols is improved. The test: `const numSamples = 10000000; const strPercents = '%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%s%%%%%%%%%%%%%%%%%i%%%%%%%%%%%%%%%%%%%%%%%%%%'; var s; console.time('Percents'); for (let i = 0; i < numSamples; i++) { s = util.format(strPercents, 'test', 12); } console.timeEnd('Percents');` Original time: 28399.708ms After refactor: 23763.788ms Improved: 16%

vsemozhetbyt · 2017-04-14T01:31:48Z

Thank you for the contribution!

CI: https://ci.nodejs.org/job/node-test-pull-request/7390/

vsemozhetbyt · 2017-04-14T01:37:28Z

lib/util.js

-            break;
-          if (lastPos < i)
-            str += f.slice(lastPos, i);
-          str += tryStringify(arguments[a++]);


It seems this function was used only here (linter reports 'tryStringify' is defined but never used)

Fixed. I commited the changes. Also passed all tests.

vsemozhetbyt · 2017-04-14T01:38:28Z

lib/util.js

          lastPos = i = i + 2;
          continue;
-      }
+        }
+      }      


Linter: Trailing spaces not allowed

vsemozhetbyt · 2017-04-14T01:47:18Z

test/parallel/test-util-format.js fails with TypeError: Converting circular structure to JSON. See now abandoned tryStringify().

Did you run tests for a new build with your fix?

hiroppy · 2017-04-14T02:03:32Z

Thanks for your pull request.

I think you should write a test before refactoring.
Required lines: L91, 93, 99, 101
coverage: https://coverage.nodejs.org/coverage-d0cd8c8589173da1/root/util.js.html

mscdex · 2017-04-14T04:05:17Z

These changes as-is introduce performance regressions:

                                           improvement confidence      p.value
 util/format.js type="number" n=10000000       2.32 %        *** 1.138891e-05
 util/format.js type="object" n=10000000      -5.17 %        *** 1.038111e-06
 util/format.js type="string" n=10000000      -2.35 %        *** 2.043293e-07
 util/format.js type="unknown" n=10000000     -8.66 %        *** 4.520538e-26

I didn't bother running with type=no-replace because it takes considerably longer than the others.

I believe similar changes had been suggested in the past (although with a lookup table instead of a function with a switch) and there were performance regressions back then as well. I'm not sure if (or how much) inlining the helper function would make a positive difference.

vsemozhetbyt · 2017-04-14T09:14:27Z

CI: https://ci.nodejs.org/job/node-test-pull-request/7399/

jseijas · 2017-04-14T09:20:04Z

I updated the PR with these changes:

Added unit tests for having full coverage before refactor, as asked by @abouthiroppy
Added tryStringify and run the unit tests, as asked by @vsemozhetbyt (Sorry! It was my fault!)

About performance asked by @mscdex:
I tried with the benchmark in both Linux (Ubuntu 64) and Windows (Windows 7 64). The results with previous master and this PR are pretty similar.

Previous master:

util/format.js type="string" n=1000000: 1,855,295.8547256442
util/format.js type="number" n=1000000: 1,830,136.1041657394
util/format.js type="object" n=1000000: 253,222.17449454102
util/format.js type="unknown" n=1000000: 3,623,680.543628614
util/format.js type="no-replace" n=1000000: 30,053.1634236955

This PR

util/format.js type="string" n=1000000: 1,891,075.1743455955
util/format.js type="number" n=1000000: 1,564,578.625822645
util/format.js type="object" n=1000000: 236,045.4131518764
util/format.js type="unknown" n=1000000: 3,472,115.0568377078
util/format.js type="no-replace" n=1000000: 30,129.379419319466

Also I checked with my own script to compare between current master and this PR. This is the script that I used:

const util = require('util');
const numSamples = 10000000;

const strShort = 'a';
const strNormal = 'Name is %s number is %i. Done.';
const strPercents = '%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%s%%%%%%%%%%%%%%%%%i%%%%%%%%%%%%%%%%%%%%%%%%%%';
var s;

console.time('Short');
for (let i = 0; i < numSamples; i++) {
  s = util.format(strShort);
}
console.log(s);
console.log('Time for '+numSamples+' samples of Short');
console.timeEnd('Short');

console.time('Normal');
for (let i = 0; i < numSamples; i++) {
  s = util.format(strNormal, 'test', 12);
}
console.log(s);
console.log('Time for '+numSamples+' samples of Normal');
console.timeEnd('Normal');

console.time('Percents');
for (let i = 0; i < numSamples; i++) {
  s = util.format(strPercents, 'test', 12);
}
console.timeEnd('Percents');

console.time('Less Parameters');
for (let i = 0; i < numSamples; i++) {
  s = util.format(strNormal, 'test');
}
console.log(s);
console.log('Time for '+numSamples+' samples of Less Parameters');
console.timeEnd('Less Parameters');

And these are the results with my script:

Previous master

Time for 10000000 samples of Short
Short: 382.084ms
Time for 10000000 samples of Normal
Normal: 11162.574ms
Percents: 27439.185ms
Time for 10000000 samples of Less Parameters
Less Parameters: 8967.359ms

This PR:

Time for 10000000 samples of Short
Short: 389.939ms
Time for 10000000 samples of Normal
Normal: 11141.138ms
Percents: 23552.820ms
Time for 10000000 samples of Less Parameters
Less Parameters: 8914.957ms

After 10 different runs, I always get similar times for the Short, Normal, and Less Parameters, but for the Percents (use of '%%%%%%%%%%' inside the string) I'm getting always best performance with the PR code.

silverwind · 2017-04-14T17:04:01Z

test/parallel/test-util-format.js

 assert.strictEqual(util.format('o: %j, a: %j', {}, []), 'o: {}, a: []');
 assert.strictEqual(util.format('o: %j, a: %j', {}), 'o: {}, a: %j');
 assert.strictEqual(util.format('o: %j, a: %j'), 'o: %j, a: %j');

+


Nit: unneccesary newline.

silverwind

LGTM if perf is the same.

benjamingr

Changes LGTM, I'm fine with a ~5% regression in perf of util.format since it's not used in "hot paths" of node apps anyway and I think the new code is a lot more readable.

mscdex · 2017-04-14T18:44:43Z

@benjamingr I wouldn't say it's not in a "hot path." I would bet at least logging modules and such make use of it. I would also bet that having consecutive %'s like that is uncommon.

jseijas · 2017-04-14T18:52:48Z

@mscdex You're right, consecutive %'s is uncommon. But the objective of this PR is not to improve the performance, as you can read in the description the objective is to make it more maintenable. I checked the perf in a Linux, a Windows and a docker, and I'm not able to reproduce the -5%. If you provide me a way to reproduce it I will put my hands on it! :)

Also, the final target of this PR is, perhaps in the future, add more usable formats (%u, %o, %x, %X, %e and %E) without having duplicated code go total nightmare.

benjamingr · 2017-04-14T18:57:04Z

@mscdex

@benjamingr I wouldn't say it's not in a "hot path." I would bet at least logging modules and such make use of it.

I was under the impression logging modules are never in hot paths anyway - but I'm ready to be educated :)

mscdex · 2017-04-14T19:19:54Z

@benjamingr Obviously it depends on the application. It could easily be a bigger issue when you enable extensive debugging in your application in production (even temporarily). You could make a similar argument for other node core modules/functions that may/may not be directly used by core itself (e.g. querystring or url) but are used extensively in userland.

@jseijas Here's what I used:

node benchmark/compare.js --new ./node --old ./node-master --filter format.js --set n=10000000 --set type=unknown --set type=number --set type=object --set type=string util | Rscript benchmark/compare.R

Where ./node is a node master binary with the changes from this PR and node-master is current master without these changes. You will need R installed.

Improved performance

jseijas · 2017-04-15T03:39:05Z

@mscdex Changed the way of doing the refactor. With the new way:

                                           improvement confidence      p.value
 util/format.js type="number" n=10000000      15.02 %        *** 3.135523e-13
 util/format.js type="object" n=10000000      -4.93 %        *** 2.037599e-31
 util/format.js type="string" n=10000000      14.36 %        *** 1.535491e-23
 util/format.js type="unknown" n=10000000      6.60 %        *** 1.233373e-30

I don't understand why for %j the performance is decreased. But also while refactoring I found some odd behaviours with the performance, example: if (f.charCodeAt(i+1) === 37 || a < argLen) if I swap the order of the conditions then the performance is decreased.

mscdex · 2017-04-15T04:05:23Z

@jseijas It's hard to say why offhand without running the benchmark with type=object through irhydra2 to see what V8 is doing compared to the other types.

I would expect it to be somewhat slower (but not necessarily a regression) because the object type case is farther down in the switch and V8 matches each in order.

vsemozhetbyt · 2017-04-15T12:31:53Z

CI: https://ci.nodejs.org/job/node-test-pull-request/7416/

vsemozhetbyt · 2017-04-15T12:36:46Z

lib/util.js

-          str += '%';
-          lastPos = i = i + 2;
-          continue;
+      if (f.charCodeAt(i+1) === 37 || a < argLen) {


Linter: Infix operators must be spaced

Fix lint error: Infix operators must be spaced

benjamingr · 2017-04-15T13:15:19Z

@mscdex

Obviously it depends on the application. It could easily be a bigger issue when you enable extensive debugging in your application in production (even temporarily).

I don't know - I just want to see a case where the performance is meaningful in util.format. Even a single telemetry tool that logs requests and uses util.format would convince me. I admit I have far less experience with these sort of workloads though.

You could make a similar argument for other node core modules/functions that may/may not be directly used by core itself (e.g. querystring or url) but are used extensively in userland.

I might be seeing this the wrong way - but util.format is a debugging utility whereas the querystring/url modules are typically used per request in every request in most http servers I've seen.

@jseijas

if (f.charCodeAt(i+1) === 37 || a < argLen) if I swap the order of the conditions then the performance is decreased.

charCodeAt is special and optimized to a simple value check. Remember that in == when the first condition is true it can skip the other condition. In this case it is possible the left hand side is true a lot more often then the right hand side in the benchmark and a conditional check is saved - even if the JIT is being stupid.

Of course, a faster implementation would parse the format string once and then compile it to a function with Function and store it in a cache (so the parsing only happens once in the first util.inspect call with the format string) and then subsequent calls don't need to do any parsing. This is what templating engines typically do - but I doubt it's worth it since I'm still not convinced performance matters here.

vsemozhetbyt · 2017-04-15T13:29:20Z

CI: https://ci.nodejs.org/job/node-test-pull-request/7417/

jseijas · 2017-04-15T13:51:00Z

@benjamingr Well, I just was doing several changes and learning how they affect to the performance. I'm with you, I never considered util.format as critical, I consider it more like an utility, and I think that could be interesting to evolve it to take into account the different formats of "printf". I think that can be interesting to have at least %e and %x provided by the core, without installing any other package. Even more interesting to have things like "%8.2f", but in this case a deeper modification of the function is needed.

But if we think "ok, it's critical, we are using it for the logs of the app", then we can see that we got it improved a 15% for numbers and strings (the usual case of logs). For those that wants to stringify objects inside the messages, we are talking of a -5% performance, in my laptop it means 0.0001ms per message... then I have to store this log, so I don't think that this time is really relevant if we consider the I/O operation for storing. We can consider it relevant if our system is storing huge amounts of logs per second, but in this case I think there are a bunch of problems at architecture level of the app before even looking to this 5%.

But well, is my point of view ^^

evanlucas · 2017-04-15T15:09:51Z

I would prefer we not knowingly allow a performance regression to util.format(). It is used in things like debug (which is used in various places throughout the ecosystem). If we can make this have at least the same performance, then +1.

jseijas · 2017-04-16T04:33:35Z

Ok, I changed the code to focus on performance. Is still more readable than the original one, but now the performance is:

                                           improvement confidence      p.value
 util/format.js type="number" n=10000000       8.08 %        *** 1.032080e-10
 util/format.js type="object" n=10000000       8.37 %        *** 6.609044e-38
 util/format.js type="string" n=10000000       4.28 %        *** 6.155360e-08
 util/format.js type="unknown" n=10000000      4.08 %        *** 5.374915e-20

So we have more readable code, and with better performance. Yay!

Even with that, I'm not happy. I want to say:

If we consider it critical because is used to log, then we should measure the performance using the console.log(util.format(...)). A 5% of performance in util.format() means a 0.3% over console.log(util.format()).
The problem with performance at this granularity level is that something so simple as creating a variable has an impact in the performance, because we are measuring a very small process that is pretty fast. Several examples at this code:

I removed the variable argLen, because the code is faster;
At the line if (f.charCodeAt(i + 1) !== 37 && a >= arguments.length), if you swap the parameters, you have a ~5% less performance
If you look at each case of the switch you'll see the same repeated code: if (i > lastPos) str += f.slice(lastPos, i);. If this code is extracted out of the switch to avoid repeating it, you get a -10% penalization in the %j. This is a bit weird.

Forgive me Robert Cecil Martin, because I have sinned.

jseijas · 2017-04-16T10:45:16Z

One thing that I don't understand:

There is a real problem with the no-replace performance, and is due to the fact that the behaviour of util.format(1,2) is totally different from util.format('', 1, 2), even returning the same result. If we take a look into the code the first is resolved with

  const objects = new Array(arguments.length);
    for (var index = 0; index < arguments.length; index++) {
      objects[index] = inspect(arguments[index]);
    }
    return objects.join(' ');

and the second one with:

  while (a < argLen) {
    const x = arguments[a++];
    if (x === null || (typeof x !== 'object' && typeof x !== 'symbol')) {
      str += ' ' + x;
    } else {
      str += ' ' + inspect(x);
    }
  }

Where we see that the inspect is only called when the object is not null, and is an object or symbol, so no inspect will be called for strings or numbers.

That's the reason why no-replace benchmark takes so much time compared with the other options.

Replacing the first part of the util.format with this:

  var a = 0;
  var str = '';
  if (typeof f !== 'string') {
    while (a < arguments.length) {
      if (a > 0) 
        str += ' ';
      const x = arguments[a++];
      if (x === null || (typeof x !== 'object' && typeof x !== 'symbol')) {
        str += x;
      } else {
        str += inspect(x);
      }
    }
    return str;
  }

then we get this benchmark result for no-replace:

                                           improvement confidence      p.value
util/format.js type="no-replace" n=1000000  16651.20 %        *** 1.225682e-52

@mscdex Is this fix good, or it something that I'm not taking into account and there is a good reason for doing inspect for all the parameters only at the no-replace part? I think that the differences should be the stylize, the "-0" case, and the replacements done in strings. But if we are defending that util.format is a so critical part, then performance should go before beauty. And we are talking about 16651% of performance.

aqrln · 2017-04-16T11:55:44Z

CI: https://ci.nodejs.org/job/node-test-pull-request/7425/

refack · 2017-04-16T12:39:10Z

[Silly comment] how about adding util.formatEx for more formats, with the implicit assumption it will be less performant than util.format?
@jseijas does that make sense code-reuse-wise?

mscdex · 2017-04-16T15:55:47Z

@jseijas I think backwards compatibility should be preferred since then more branches can receive at least some amount of increased performance, but if backwards compatibility can't be met and you feel the performance improvement is great enough and is worth marking as semver-major, then so be it.

@refack I don't think getting into that kind of habit with the node API is a good goal. If anything it's usually a sign that the changes should just go into a userland module instead.

jseijas · 2017-04-16T19:13:14Z

@mscdex Ok, then I'll make this change ina new PR, if this one is merged, because currently this one has not backwards impact. And also are different topics. To give more information about how critical is the impact that this have, running console.log(util.format(1,2,3,4)); 1M times, takes 109664ms with the current master, and 44367ms after patching this code, generating exactly the same log. This means that currently the util.format is taking more time than the I/O operation.

jasnell · 2017-04-17T17:25:43Z

LGTM but I'd like @mscdex to sign off before it lands

addaleax

Fresh CI: https://ci.nodejs.org/job/node-test-commit/9516/

I’m good with landing this as it is once CI comes back good.

jseijas · 2017-05-23T10:49:47Z

Hi @mscdex! Do you need more changes to be made, or you are ok with this? Or any other suggestion.

mscdex · 2017-05-31T12:59:12Z

lib/util.js


  var str = '';
  var a = 1;
  var lastPos = 0;
  for (var i = 0; i < f.length;) {
-    if (f.charCodeAt(i) === 37/*'%'*/ && i + 1 < f.length) {
+    if (f.charCodeAt(i) === 37/*'%'*/ && f.length > i + 1) {


I think having the "static" portion on the right-hand side is more traditional.

mscdex · 2017-05-31T12:59:44Z

lib/util.js


  var str = '';
  var a = 1;
  var lastPos = 0;
  for (var i = 0; i < f.length;) {
-    if (f.charCodeAt(i) === 37/*'%'*/ && i + 1 < f.length) {
+    if (f.charCodeAt(i) === 37/*'%'*/ && f.length > i + 1) {
+      if (f.charCodeAt(i + 1) !== 37 && a >= arguments.length) {


The /*'%'*/ is missing here for consistency.

mscdex · 2017-05-31T13:00:15Z

lib/util.js

@@ -143,7 +131,6 @@ exports.format = function(f) {
  return str;
 };

-


Unintended whitespace change?

mscdex · 2017-05-31T13:01:42Z

lib/util.js

      switch (f.charCodeAt(i + 1)) {
        case 100: // 'd'
-          if (a >= argLen)
-            break;
-          if (lastPos < i)


I prefer this style because lastPos will always be either less than or equal to i.

Sorry, I didn't understand this point. You meain only remain lastPos < i instead of i > lastPos ? Thank you!

I mean I prefer lastPos < i

- Correct whitespace - Static parts of if before unstatic.

jseijas · 2017-05-31T18:54:24Z

@mscdex Changes done!

jseijas · 2017-05-31T23:42:56Z

After rebase:

                                           improvement confidence      p.value
 util/format.js type="number" n=10000000       6.58 %        *** 5.428519e-11
 util/format.js type="object" n=10000000       0.18 %            6.416948e-01
 util/format.js type="string" n=10000000      10.19 %        *** 3.134553e-23
 util/format.js type="unknown" n=10000000      9.45 %        *** 6.444361e-28

silverwind · 2017-06-01T04:32:07Z

Another CI: https://ci.nodejs.org/job/node-test-pull-request/8396/

mscdex · 2017-06-01T20:53:35Z

LGTM

silverwind · 2017-06-02T13:36:07Z

Thanks! Landed in 4191962.

Method format refactored to make it more maintenable, replacing the switch by a function factory, that returns the appropiated function given the character (d, i , f, j, s). Also, performance when formatting an string that contains several consecutive % symbols is improved. The test: `const numSamples = 10000000; const strPercents = '%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%s%%%%%%%%%%%%%%%%%i%%%%%%%%%%%%%%%%%%%%%%%%%%'; var s; console.time('Percents'); for (let i = 0; i < numSamples; i++) { s = util.format(strPercents, 'test', 12); } console.timeEnd('Percents');` Original time: 28399.708ms After refactor: 23763.788ms Improved: 16% PR-URL: #12407 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Brian White <mscdex@mscdex.net>

mscdex · 2017-06-13T23:16:06Z

FWIW it looks like this could be causing issues due to the change in behavior (see #13665), so I've marked this as semver-major now.

nodejs-github-bot added the util Issues and PRs related to the built-in util module. label Apr 14, 2017

vsemozhetbyt reviewed Apr 14, 2017

View reviewed changes

lib/util.js Outdated

lastPos = i = i + 2;

continue;

}

}

}

Copy link

Contributor

vsemozhetbyt Apr 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linter: Trailing spaces not allowed

refack force-pushed the master branch from 16073c0 to fbe946b Compare April 14, 2017 04:12

util: Added tests and fix bugs

181ec8a

silverwind reviewed Apr 14, 2017

View reviewed changes

silverwind approved these changes Apr 14, 2017

View reviewed changes

Removed unnecesary newline

aa30bb2

benjamingr approved these changes Apr 14, 2017

View reviewed changes

util: refactor format method.

bf6f8fe

Improved performance

vsemozhetbyt reviewed Apr 15, 2017

View reviewed changes

util: refactor format method.

9d032c2

Fix lint error: Infix operators must be spaced

Fix lint

78118c7

addaleax approved these changes Apr 30, 2017

View reviewed changes

addaleax requested a review from mscdex April 30, 2017 00:02

mscdex reviewed May 31, 2017

View reviewed changes

lib/util.js Outdated

@@ -143,7 +131,6 @@ exports.format = function(f) {

return str;

};

Copy link

Contributor

mscdex May 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unintended whitespace change?

mscdex reviewed May 31, 2017

View reviewed changes

util: refactor format method

9f762cf

- Correct whitespace - Static parts of if before unstatic.

jasnell approved these changes Jun 1, 2017

View reviewed changes

silverwind closed this Jun 2, 2017

jasnell mentioned this pull request Jun 5, 2017

8.1.0 Proposal #13483

Closed

mscdex added the semver-major PRs that contain breaking changes and should be released in the next major version. label Jun 13, 2017

mscdex mentioned this pull request Jun 13, 2017

util.format not printing % when there's multiple arguments #13665

Closed

targos mentioned this pull request Jun 14, 2017

util: ignore invalid format specifiers #13674

Closed

3 tasks

		@@ -143,7 +131,6 @@ exports.format = function(f) {
		return str;
		};

util: refactor format method. #12407

util: refactor format method. #12407

Conversation

jseijas commented Apr 14, 2017 • edited Loading

Checklist

Affected core subsystem(s)

vsemozhetbyt commented Apr 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vsemozhetbyt commented Apr 14, 2017 • edited Loading

hiroppy commented Apr 14, 2017

mscdex commented Apr 14, 2017 • edited Loading

vsemozhetbyt commented Apr 14, 2017

jseijas commented Apr 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

silverwind left a comment

Choose a reason for hiding this comment

benjamingr left a comment

Choose a reason for hiding this comment

mscdex commented Apr 14, 2017

jseijas commented Apr 14, 2017

benjamingr commented Apr 14, 2017

mscdex commented Apr 14, 2017 • edited Loading

jseijas commented Apr 15, 2017

mscdex commented Apr 15, 2017 • edited Loading

vsemozhetbyt commented Apr 15, 2017

Choose a reason for hiding this comment

benjamingr commented Apr 15, 2017

vsemozhetbyt commented Apr 15, 2017

jseijas commented Apr 15, 2017

evanlucas commented Apr 15, 2017

jseijas commented Apr 16, 2017

jseijas commented Apr 16, 2017 • edited by gibfahn Loading

aqrln commented Apr 16, 2017

refack commented Apr 16, 2017 • edited Loading

mscdex commented Apr 16, 2017 • edited Loading

jseijas commented Apr 16, 2017

jasnell commented Apr 17, 2017

addaleax left a comment

Choose a reason for hiding this comment

jseijas commented May 23, 2017

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

jseijas commented May 31, 2017

jseijas commented May 31, 2017

silverwind commented Jun 1, 2017

mscdex commented Jun 1, 2017

silverwind commented Jun 2, 2017

mscdex commented Jun 13, 2017

jseijas commented Apr 14, 2017 •

edited

Loading

vsemozhetbyt commented Apr 14, 2017 •

edited

Loading

mscdex commented Apr 14, 2017 •

edited

Loading

jseijas commented Apr 14, 2017 •

edited

Loading

mscdex commented Apr 14, 2017 •

edited

Loading

mscdex commented Apr 15, 2017 •

edited

Loading

jseijas commented Apr 16, 2017 •

edited by gibfahn

Loading

refack commented Apr 16, 2017 •

edited

Loading

mscdex commented Apr 16, 2017 •

edited

Loading