Re: [racket-users] Re: Typed Racket has lowered my performance

[HiPhish]

No, I did the same thing, and only the first hundred tests work normally, 
the other 100 hand for several minutes. Maybe my computer is too weak, it's 
an early 2009 iMac with a 2.66GHz Core2Duo and 8GB of RAM. I also ran `raco 
setup msgpack` after making the change to the source file to make sure 
everything gets compiled properly.

On Monday, December 4, 2017 at 11:59:10 PM UTC+1, Sam Tobin-Hochstadt wrote:
>
> I'm not able to replicate that (see transcript below). Is there 
> something else I should be doing? 
>

-- 
You received this message because you are subscribed to the Google Groups 
"Racket Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to racket-users+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [racket-users] Re: Typed Racket has lowered my performance

[Sam Tobin-Hochstadt]

I'm not able to replicate that (see transcript below). Is there
something else I should be doing?

[samth@huor:~/tmp/MsgPack.rkt (master) plt] git diff
diff --git a/unpack.rkt b/unpack.rkt
index 30c87a9..3806548 100644
--- a/unpack.rkt
+++ b/unpack.rkt
@@ -81,7 +81,7 @@
  [else (error "Unknown tag " tag-var)])]))

 (: unpack (-> Input-Port
-  (U Void Boolean Integer Real String Bytes (Vectorof
Packable) (Listof Packable) (HashTable Packable Packable) Ext)))
+  Packable))
 (define (unpack in)
   (define tag (read-byte in))
   (cond
[samth@huor:~/tmp/MsgPack.rkt (master) plt] racket -l msgpack/test/pack/map
[samth@huor:~/tmp/MsgPack.rkt (master) plt] raco test -l msgpack/test/pack/map
raco test: (submod "/home/samth/tmp/MsgPack.rkt/test/pack/map.rkt" test)
OK, passed 100 tests.
OK, passed 100 tests.
2 tests passed



On Mon, Dec 4, 2017 at 6:56 AM, HiPhish  wrote:
> When I change the return type of `unpack` to `Packable` instead of an
> explicit union of types the map packing test (`test/pack/map.rkt`) hangs.
> https://gitlab.com/HiPhish/MsgPack.rkt/blob/master/unpack.rkt#L83
> https://gitlab.com/HiPhish/MsgPack.rkt/blob/master/test/pack/map.rkt
>
> --
> You received this message because you are subscribed to the Google Groups
> "Racket Users" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to racket-users+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups 
"Racket Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to racket-users+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [racket-users] Re: Typed Racket has lowered my performance

[HiPhish]

When I change the return type of `unpack` to `Packable` instead of an 
explicit union of types the map packing test (`test/pack/map.rkt`) hangs.
https://gitlab.com/HiPhish/MsgPack.rkt/blob/master/unpack.rkt#L83
https://gitlab.com/HiPhish/MsgPack.rkt/blob/master/test/pack/map.rkt

-- 
You received this message because you are subscribed to the Google Groups 
"Racket Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to racket-users+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [racket-users] Re: Typed Racket has lowered my performance

[Sam Tobin-Hochstadt]

Ok, looking more at the commit, I think this is not actually a bug
anywhere, but really an unfortunate combination of things that I don't
have an idea for improving at the moment. Sorry for not realizing that
earlier.

First, I think you should just go back to using `Any` in the type for
`pack` and for `pack-hash` and `pack-sequence`. I don't think that
will cause any problems for you or users of your code, and will make
the performance problem go away. For functions where `Packable`
appears in the _result_, such as `unpack`, you should keep using
`Packable` -- that will be both faster and will avoid contract errors
that your users might otherwise encounter.

More generally, what happened here was that you made the types you
export _more restrictive_ by changing `Any` to `Packable` in the
argument types. That is, in the old code, anyone could pass any value
to `pack` and either get the result or a dynamic error. That means
that Typed Racket could generate a very cheap contract for `pack`. By
changing it to a more restrictive type, you get to assume in the body
of `pack` that the input is `Packable`, but Typed Racket then
generates a very expensive contract to check that. From your code, it
doesn't look like you're making use of that additional assumption, so
it's just costing you a lot of performance.

As to why that contract is so expensive, the short answer is that
contracts for mutable data like procedures and hash tables have to
construct wrapper objects, which involves a lot of extra allocation
and indirection, on top of the usual expense of contract checking.
That's why things got faster with Ben's modifications.

Finally, why use `Packable` in the result type of `unpack`? Here, the
contract for `Any` isn't simple and inexpensive, since you're sharing
potentially arbitrary values with untyped code, so Typed Racket
constructs a complicated contract (called `any-wrap/c`) in order to
protect it. That contract will also error in cases where it doesn't
know what to do, which the contract for `Packable` won't.

Sam

On Sun, Dec 3, 2017 at 6:53 PM, HiPhish  wrote:
> Anything more I can do?
> On Sunday, December 3, 2017 at 6:11:42 PM UTC+1, Sam Tobin-Hochstadt wrote:
>>
>> Thanks, that's very helpful. It's clear that the contract optimization is
>> working in the old code but not the new code, and we need to fix that.
>
> --
> You received this message because you are subscribed to the Google Groups
> "Racket Users" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to racket-users+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups 
"Racket Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to racket-users+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [racket-users] Re: Typed Racket has lowered my performance

[HiPhish]

Anything more I can do?
On Sunday, December 3, 2017 at 6:11:42 PM UTC+1, Sam Tobin-Hochstadt wrote:
>
> Thanks, that's very helpful. It's clear that the contract optimization is 
> working in the old code but not the new code, and we need to fix that.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Racket Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to racket-users+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [racket-users] Re: Typed Racket has lowered my performance

[Sam Tobin-Hochstadt]

Thanks, that's very helpful. It's clear that the contract optimization is
working in the old code but not the new code, and we need to fix that.

Sam

On Dec 3, 2017 12:07 PM, "HiPhish"  wrote:

> Here is what happens when I run one of the array tests with the more
> restrictive type specifications:
>
> OK, passed 100 tests.
> Running time is 70.75% contracts
> 75/106 ms
>
> (-> (recursive-contract (or/c (and/c hash? (and/c hash-equal ... 75 ms
> (lib msgpack/pack.rkt):24:9
> pack 75 ms
>
> After reverting the commit I get zero overhead:
>
> OK, passed 100 tests.
> Running time is 0% contracts
> 0/7 ms
>
> The contract makes up 70% of the total runtime. I also looks like there is
> no
> contract generated after reverting. Should I run the profiler on some other
> tests as well?
>
> On Sunday, December 3, 2017 at 3:33:10 PM UTC+1, Sam Tobin-Hochstadt wrote:
>>
>> Running the contract profiler [1] on your code would be quite helpful.
>>
>> [1] https://docs.racket-lang.org/contract-profile/index.html
>>
>> Sam
>>
> --
> You received this message because you are subscribed to the Google Groups
> "Racket Users" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to racket-users+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Racket Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to racket-users+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [racket-users] Re: Typed Racket has lowered my performance

[HiPhish]

Here is what happens when I run one of the array tests with the more
restrictive type specifications:

OK, passed 100 tests.
Running time is 70.75% contracts
75/106 ms

(-> (recursive-contract (or/c (and/c hash? (and/c hash-equal ... 75 ms
(lib msgpack/pack.rkt):24:9
pack 75 ms

After reverting the commit I get zero overhead:

OK, passed 100 tests.
Running time is 0% contracts
0/7 ms

The contract makes up 70% of the total runtime. I also looks like there is 
no
contract generated after reverting. Should I run the profiler on some other
tests as well?

On Sunday, December 3, 2017 at 3:33:10 PM UTC+1, Sam Tobin-Hochstadt wrote:
>
> Running the contract profiler [1] on your code would be quite helpful. 
>
> [1] https://docs.racket-lang.org/contract-profile/index.html 
>
> Sam 
>

-- 
You received this message because you are subscribed to the Google Groups 
"Racket Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to racket-users+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [racket-users] Re: Typed Racket has lowered my performance

[Sam Tobin-Hochstadt]

Running the contract profiler [1] on your code would be quite helpful.

[1] https://docs.racket-lang.org/contract-profile/index.html

Sam

On Sun, Dec 3, 2017 at 9:29 AM, HiPhish  wrote:
> Is there anything I can do to help investigate the issue? I have reverted my
> commit for the time being, and it's a difference like day and night.
>
> On Sunday, December 3, 2017 at 12:36:16 AM UTC+1, Sam Tobin-Hochstadt wrote:
>>
>> I don't think the mutable/immutable issue should be as significant as it
>> seems here. Using the Any type shouldn't perform better the way you
>> describe, so we need to look more at what the actual contracts are doing.
>>
>> Sam
>
> --
> You received this message because you are subscribed to the Google Groups
> "Racket Users" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to racket-users+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups 
"Racket Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to racket-users+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [racket-users] Re: Typed Racket has lowered my performance

[HiPhish]

Is there anything I can do to help investigate the issue? I have reverted 
my commit for the time being, and it's a difference like day and night.

On Sunday, December 3, 2017 at 12:36:16 AM UTC+1, Sam Tobin-Hochstadt wrote:
>
> I don't think the mutable/immutable issue should be as significant as it 
> seems here. Using the Any type shouldn't perform better the way you 
> describe, so we need to look more at what the actual contracts are doing.
>
> Sam
>

-- 
You received this message because you are subscribed to the Google Groups 
"Racket Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to racket-users+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [racket-users] Re: Typed Racket has lowered my performance

[Sam Tobin-Hochstadt]

I don't think the mutable/immutable issue should be as significant as it
seems here. Using the Any type shouldn't perform better the way you
describe, so we need to look more at what the actual contracts are doing.

Sam

On Dec 2, 2017 6:24 PM, "HiPhish"  wrote:

> Now that I think about it, changing the types to be immutable is not really
> correct either. There is no reason users should not be able to serialise a
> mutable list, vector or hash table, just as they can serialise any mutable
> scalar as well.
>
> The result of unpacking bytes could be immutable, but would that make any
> difference?
>
> --
> You received this message because you are subscribed to the Google Groups
> "Racket Users" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to racket-users+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Racket Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to racket-users+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[racket-users] Re: Typed Racket has lowered my performance

[HiPhish]

Now that I think about it, changing the types to be immutable is not really
correct either. There is no reason users should not be able to serialise a
mutable list, vector or hash table, just as they can serialise any mutable
scalar as well.

The result of unpacking bytes could be immutable, but would that make any
difference?

-- 
You received this message because you are subscribed to the Google Groups 
"Racket Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to racket-users+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.