nocost: rename fancyflow_trans + turn [pipe] & [maybe] pipes into zero-cost abstractions #2

fenollp · 2018-06-18T14:23:23Z

This PR has 2 goals:

allowing user-defined pipes, e.g.: [{folder, knm_numbers, pipe}](T,F1,F2,F3)
making the fancyflow-provided pipes abstractions runtime cost less than a remote call down to zero if possible.

Right now the parse transform only replaces the pipes with function calls.
Since the number of arguments to a pipe is known at compile time, I'd like to inline at least the fancyflow-provided pipes, with proper mapping of line numbers to function calls. The simplest one being fancyflow:pipe/2.

Do you think it is possible to make use of https://github.com/erlang/otp/blob/9d5af99762b3795c763fb62c1516247bd3f8e12f/lib/compiler/src/sys_core_fold_lists.erl#L209 or should I just be looking into manually inlining the fancyflow-provided pipes?

Note: I introduced 2 pipe kinds (mapper & folder). It should now be possible to no longer need to give undefined as a first arg for mapper pipes, but I have not looked into it yet.

ferd · 2018-06-18T20:12:08Z

I'll have to think about it. I'm deeply uncomfortable with people relying on these constructs in any kind of production code. They were experiments to explore some semantics for pipe and pipe-equivalence in Erlang, and the library is clearly marked as experimental. It's part of the reason I have refused to publish it on Hex so far.

So seeing steps that seem towards making these things more production-friendly kind of rubs me the wrong way with regards to the spirit of this library. Specifically, letting user-specified pipes get in on this just makes everything over-rely on the shitty bad syntax chosen ([Desc](A, B, C)) and rather than adding flexibility here, it would be much nicer to have something better that does not rely on the compiler being way too lax about some of the things it allows.

I'm not too sure what [{folder,knm_numbers,pipe}](T,F1,F2,F3) means. Does it apply folder:knm_numbers with pipe semantics, or is it a folder type of pipe applying knm_numbers:pipe. If the latter, why not just use a regular fold?

fenollp · 2018-06-18T20:38:07Z

Oh I'm not using it at all. I am just experimenting with it, trying to learn some lessons to some day write an informed EEP and maybe get a pipe operator into Erlang :)

The syntax hack really doesn't matter and it's just "temporary".

Yes, with [{folder,knm_numbers,pipe}](T,F1,F2,F3) I meant "call knm_numbers:pipe/2 as a folder pipe on the value T with functions F1, F2, F3.
The function wouldn't just be a fold underneath, like fancyflow:maybe/2 isn't.
I just want a way for people to experiment with their own pipes in an easier way.

fenollp · 2018-06-19T01:25:00Z

yay I managed to inline fancyflow:pipe! which makes this PR ready for review :)

fenollp · 2018-06-19T19:01:50Z

Ready! :)

ferd

There's a few things I'm not quite sure are properly covered here.

ferd · 2018-06-20T11:41:23Z

src/fancyflow.app.src

@@ -1,5 +1,5 @@
 {application, fancyflow,
- [{description, "Experimental library to bring pipe and maybe operator equivalents in Erlang"},
+ [{description, "Experimental library to bring pipe and maybe operator equivalents to Erlang"},
  {vsn, "0.1.0"},


should probably come with a version bump

ferd · 2018-06-20T11:43:10Z

src/fancyflow.erl

+op_new({atom,_,maybe}) -> #op{kind=folder, mname=?MODULE, fname=maybe};
+op_new({atom,_,parallel}) -> #op{kind=mapper, mname=?MODULE, fname=parallel};
+op_new({tuple, _, [{atom,_,K}, {atom,_,M}, {atom,_,F}]}) ->
+    #op{kind=K, mname=M, fname=F}.


afaict this crashes hard on an unsupported form rather than just tunnelling it through

ferd · 2018-06-20T11:50:18Z

src/fancyflow.erl

+
+mixin_pipe_fold(Piped, {{var,_,LastVarName},Block}) ->
+    L = element(2, Piped),
+    Replacer = fun (T) -> replace_var(T, LastVarName) end,


I think this form of rewriting may break the cases where the var refers to an external scope thing vs. introducing a new one. In:

A = 5, [pipe](Val, begin X = A + _, X*2 end, begin X = A - _, X*3 end)

we'd expect to rewrite X to avoid exporting what is currently scoped to a function, but not A, which comes from the parent scope.

I can't figure out if the error is there, but the lack of scope analysis in the inlining of the macro makes me think it's not there.

Unless I'm getting it wrong and what this does is inline the call that the fancyflow module does, and all individual clauses are still walled within funs

Thanks for reviewing!
You are right the inlining does mess up the internal scope (I'm adding your example as a test).
However, [{K,M,F}] is not inlined (mixin-ed) and should be enclosing each expr in a fun. Turns out that also fails the new tests.

I'm looking into this.

ferd · 2018-06-20T11:53:53Z

test/fancyflow_SUITE.erl

-     pipe_trans, maybe_trans, parallel_trans,
-     mixed_trans].
+    [F || {F,1} <- ?MODULE:module_info(exports)]
+        -- [module_info, id, ok_id].


please be explicit, go back to just listing tests. Right now this would explode if init_per_suite would be added and exported.

fenollp · 2018-08-10T23:34:24Z

Got over the scoping issue with a somewhat dirty hack: a7396db

fenollp · 2018-08-13T11:03:30Z

Just added TravisCI too.

ferd · 2018-08-31T15:15:10Z

@fenollp would you like to get ownership of this repo? I think I've hit what I wanted in terms of experimentation with it, and you seem more interested in driving that stuff forwards than I am.

fenollp · 2018-09-01T09:16:28Z

Yes I’m fine with that.

ferd · 2018-09-01T11:46:02Z

heh, I can't do it as long as fenollp/fancyflow already exists

fenollp · 2018-09-01T11:52:55Z

Okay I just deleted my fork.

nocost: rename fancyflow_trans + prepare inlining of supported pipes

d57e9c1

fenollp added 2 commits June 19, 2018 00:30

nocost: mostly just having some fun here

4f56c27

nocost: test pipe execution order + inline pipe

9331427

fenollp added 3 commits June 19, 2018 14:03

nocost: handle erlang:get_stacktrace/0 deprecation

3c579cf

nocost: really inline pipe, relying on SSA, not introducing funs

f743503

nocost: also inline [maybe], in the same way

14ab013

fenollp changed the title ~~nocost: rename fancyflow_trans + prepare inlining of supported pipes~~ nocost: rename fancyflow_trans + turn [pipe] & [maybe] pipes into zero-cost abstractions Jun 19, 2018

nocost: avoid potential var name clashes

fc35490

ferd requested changes Jun 20, 2018

View reviewed changes

fenollp added 10 commits June 20, 2018 15:03

nocost: bump minor version

5c6a56f

nocost: revert all/0, no longer export maybe/2

db244f0

nocost: test foreign piping + defer wrong forms to compiler

0ed3350

propagate calling line into generated code, everywhere

c41c2b1

another way to get stacktrace

a21ab40

work around scoping issue with some hack

a7396db

more tests

1ef1a52

silence potential warnings that are only due to code generation

984647f

just use the list-call line alone: drop CLine

4ba02ab

rename Line and L to Anno

aa05a4e

activate TravisCI

987df30

fenollp closed this Aug 13, 2018

fenollp reopened this Aug 13, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nocost: rename fancyflow_trans + turn [pipe] & [maybe] pipes into zero-cost abstractions #2

nocost: rename fancyflow_trans + turn [pipe] & [maybe] pipes into zero-cost abstractions #2

fenollp commented Jun 18, 2018

ferd commented Jun 18, 2018

fenollp commented Jun 18, 2018

fenollp commented Jun 19, 2018

fenollp commented Jun 19, 2018

ferd left a comment

ferd Jun 20, 2018

ferd Jun 20, 2018

ferd Jun 20, 2018

ferd Jun 20, 2018

fenollp Jun 20, 2018

ferd Jun 20, 2018

fenollp commented Aug 10, 2018

fenollp commented Aug 13, 2018

ferd commented Aug 31, 2018

fenollp commented Sep 1, 2018

ferd commented Sep 1, 2018

fenollp commented Sep 1, 2018

nocost: rename fancyflow_trans + turn [pipe] & [maybe] pipes into zero-cost abstractions #2

Are you sure you want to change the base?

nocost: rename fancyflow_trans + turn [pipe] & [maybe] pipes into zero-cost abstractions #2

Conversation

fenollp commented Jun 18, 2018

ferd commented Jun 18, 2018

fenollp commented Jun 18, 2018

fenollp commented Jun 19, 2018

fenollp commented Jun 19, 2018

ferd left a comment

Choose a reason for hiding this comment

ferd Jun 20, 2018

Choose a reason for hiding this comment

ferd Jun 20, 2018

Choose a reason for hiding this comment

ferd Jun 20, 2018

Choose a reason for hiding this comment

ferd Jun 20, 2018

Choose a reason for hiding this comment

fenollp Jun 20, 2018

Choose a reason for hiding this comment

ferd Jun 20, 2018

Choose a reason for hiding this comment

fenollp commented Aug 10, 2018

fenollp commented Aug 13, 2018

ferd commented Aug 31, 2018

fenollp commented Sep 1, 2018

ferd commented Sep 1, 2018

fenollp commented Sep 1, 2018