pantsbuild #development

i've got a draft out with working tty support. control characters are respected, but cleanly terminating the protocol needs a bit more work. just wanted to get it posted to start running existing tests. https://github.com/pantsbuild/pants/pull/5040

rough-minister-58256

11/01/2017, 6:27 PM

sweet

average-vr-56795

11/02/2017, 3:19 PM

Ok, using futures is making me vaguely want to just make absolutely everything an Arc.

average-vr-56795

11/02/2017, 3:19 PM

Are there any good rules of thumb for when arguments should be Arcs and when a function should be Arc::new-ing things when it's given them?

average-vr-56795

11/02/2017, 3:20 PM

It feels like right now I'm just Arc-ing things when I happen to notice they need copying, and it means that my method signatures are really haphazard and inconsistent, and change fairly arbitrarily. Which I guess is ok - we control all of the callers, but... Yeah, any rules of thumb?

witty-crayon-22786

11/02/2017, 6:00 PM

@average-vr-56795: i don't know about rules of thumb there.

witty-crayon-22786

11/02/2017, 6:01 PM

but.... 1) if it's possible to just move something, prefer that, 2) if you only need one copy of a thing, maybe just clone it?, 3) if you need many copies (as in recursion), maybe prefer

Arc

witty-crayon-22786

11/02/2017, 6:02 PM

the cost of

Arc

should be pretty darn low... but i don't think i've used it very often at all in the fs code so far

witty-crayon-22786

11/02/2017, 6:02 PM

ah, but here is a good rule of thumb that makes all of this moot for now:

witty-crayon-22786

11/02/2017, 6:02 PM

"don't worry about copying things until you've benchmarked" ...?

witty-crayon-22786

11/02/2017, 6:03 PM

so, feel free to clone aggressively, and leave TODOs about considering passing `Arc`s once we've profiled

witty-crayon-22786

11/02/2017, 6:04 PM

because as it stands,

<http://fs.rs|fs.rs>

clones aggressively and doesn't use Arcs in a bunch of cases where it maybe should

witty-crayon-22786

11/02/2017, 6:05 PM

(making this up as we go along)

average-vr-56795

11/02/2017, 6:07 PM

I feel a little dirty copying everywhere when it feels like reference counting should be a nice free thing which Just Works in the world... But I'm also aware that I get sucked into different performance mindsets depending on what language I'm writing which isn't necessarily a good thing

witty-crayon-22786

11/02/2017, 6:07 PM

right... and

Arc

should be pretty darn cheap.

witty-crayon-22786

11/02/2017, 6:08 PM

so, yea... the rules of preference as above probably apply.

average-vr-56795

11/02/2017, 6:08 PM

Yeah, my problem is with the invasiveness in code, rather than the performance

witty-crayon-22786

11/02/2017, 6:08 PM

@average-vr-56795: i think this is probably why

AsRef<Path>

might be preferable

average-vr-56795

11/02/2017, 6:08 PM

"Oh, now I'm recursing in one place, so I suddenly need to change a bunch of call-sites"

witty-crayon-22786

11/02/2017, 6:09 PM

Arc<PathBuf> might dereference all the way through to Path ...?

witty-crayon-22786

11/02/2017, 6:09 PM

(not sure, but).

average-vr-56795

11/02/2017, 6:10 PM

Possibly... But then all of the clone()s you sprinkle everywhere stop being "obviously cheap" and start being "I wonder what they passed in"

witty-crayon-22786

11/02/2017, 6:10 PM

yea.

witty-crayon-22786

11/02/2017, 6:11 PM

but yes, in the spirit of "don't worry about it until you've profiled", i would say feel free to clone aggressively, and we can loop back to look at Arcs.

witty-crayon-22786

11/02/2017, 6:11 PM

(if that makes things easier)

average-vr-56795

11/02/2017, 6:11 PM

ok 🙂

witty-crayon-22786

11/02/2017, 6:12 PM

for one thing, if/when you switch to Arcs, i think only method signatures will change