darcs

Patch 1828 v3: optimizations

Title v3: optimizations
Superseder Nosy List bf
Related Issues
Status accepted Assigned To
Milestone

Created on 2019-06-15.09:04:04 by bf, last changed 2019-07-28.19:51:23 by ganesh.

Files
File name Status Uploaded Type Edit Remove
add-a-number-of-inline-or-inlinable-pragmas.dpatch bf, 2019-06-15.09:04:04 application/x-darcs-patch
document-darcs_util_hash_sha1read-and-explain-why-it-is-low_level-optimized.dpatch bf, 2019-07-28.08:11:25 application/x-darcs-patch
patch-preview.txt bf, 2019-06-15.09:04:04 text/x-darcs-patch
patch-preview.txt bf, 2019-07-28.08:11:25 text/x-darcs-patch
unnamed bf, 2019-06-15.09:04:04 text/plain
unnamed bf, 2019-07-28.08:11:25 text/plain
See mailing list archives for discussion on individual patches.
Messages
msg20820 (view) Author: bf Date: 2019-06-15.09:04:04
5 patches for repository /home/ben/src/darcs/without-v3:

patch cb7f2f109d05d80e662b019e9fb581cbea757e78
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Feb 15 12:12:30 CET 2019
  * add a number of INLINE or INLINABLE pragmas

patch e11285095b9b167b32e4ae1a160d41a69d378dcb
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Feb 16 12:37:44 CET 2019
  * optimize parsing of PrimPatchId
  
  Profiling showed that a lot of time was spent inside b16Dec when reading v3
  patches. We now read the SHA1 directly off the ByteString which is much
  faster than going via b16Dec and binary decoding. Also fail properly in the
  parse monad if the hash is malformed rather than calling error.

patch f0c8eecb250f549298d5bcb541c46f1f0d18d6e6
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Feb 22 15:00:25 CET 2019
  * optimize ctxNoConflicts

patch 1ec49cd68573933323f79b9febd4148952a5eff2
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Feb 23 12:04:28 CET 2019
  * optimize ctxAdd* by using the better fold

patch 5cedd4caaf09fd5bc04c612780ffdb547f4863d2
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Feb 23 14:00:00 CET 2019
  * clean up the foldXX mess in D.P.W.Ordered
  
  We now export only the natural folds i.e. the versions that allow lazy
  consumption of the result i.e. the one where the list constructor is
  replaced with the argument function.
Attachments
msg20821 (view) Author: bf Date: 2019-06-15.09:11:43
I added "clean up the foldXX mess in D.P.W.Ordered" because I think I 
remember there was a semantic dependency here.
msg20969 (view) Author: ganesh Date: 2019-07-26.15:40:37
> patch cb7f2f109d05d80e662b019e9fb581cbea757e78
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Fri Feb 15 12:12:30 CET 2019
>   * add a number of INLINE or INLINABLE pragmas
OK 
> patch e11285095b9b167b32e4ae1a160d41a69d378dcb
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Sat Feb 16 12:37:44 CET 2019
>   * optimize parsing of PrimPatchId
>   
>   Profiling showed that a lot of time was spent inside b16Dec when 
reading v3
>   patches. We now read the SHA1 directly off the ByteString which 
is much
>   faster than going via b16Dec and binary decoding. Also fail 
properly in the
>   parse monad if the hash is malformed rather than calling error.

OK (a comment in the code about needing an optimised implementation 
might be worthwhile)

> patch f0c8eecb250f549298d5bcb541c46f1f0d18d6e6
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Fri Feb 22 15:00:25 CET 2019
>   * optimize ctxNoConflicts

You removed the TODO about testing against the specification, is 
that test no longer wanted?

> patch 1ec49cd68573933323f79b9febd4148952a5eff2
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Sat Feb 23 12:04:28 CET 2019
>   * optimize ctxAdd* by using the better fold

OK

> patch 5cedd4caaf09fd5bc04c612780ffdb547f4863d2
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Sat Feb 23 14:00:00 CET 2019
>   * clean up the foldXX mess in D.P.W.Ordered
>   
>   We now export only the natural folds i.e. the versions that 
allow lazy
>   consumption of the result i.e. the one where the list 
constructor is
>   replaced with the argument function.

OK
msg20977 (view) Author: bf Date: 2019-07-28.08:11:25
Following up on review.

1 patch for repository http://darcs.net/screened:

patch 0221d9c78fa1d782d94d4b2d39ffbeb3bd102934
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Jul 28 10:10:24 CEST 2019
  * document Darcs.Util.Hash.sha1Read and explain why it is low-level optimized
Attachments
msg20978 (view) Author: bf Date: 2019-07-28.08:40:37
>>   * optimize parsing of PrimPatchId
>>   
> OK (a comment in the code about needing an optimised implementation 
> might be worthwhile)

Good point. See my follow-up patch.

>>   * optimize ctxNoConflicts
> 
> You removed the TODO about testing against the specification, is 
> that test no longer wanted?
Yes. The original definition from the paper is quite technical, poorly
motivated, and also missing a special case for equal contexted patches
(which do not conflict but would, according to the definition)). My
specification is more high-level and, I think, easier to understand.
This means it doesn't make much sense to show / test equivalence here.
(Though it is not hard to see why the two definitions are equivalent,
modulo the missing case.)
msg20996 (view) Author: ganesh Date: 2019-07-28.19:51:23
Thanks, I've pushed the patch with the comment.
History
Date User Action Args
2019-06-15 09:04:04bfcreate
2019-06-15 09:11:44bfsetstatus: needs-screening -> needs-review
messages: + msg20821
2019-07-26 15:40:38ganeshsetstatus: needs-review -> accepted-pending-tests
messages: + msg20969
2019-07-27 17:59:40ganeshsetstatus: accepted-pending-tests -> accepted
2019-07-28 08:11:25bfsetfiles: + patch-preview.txt, document-darcs_util_hash_sha1read-and-explain-why-it-is-low_level-optimized.dpatch, unnamed
messages: + msg20977
2019-07-28 08:40:38bfsetmessages: + msg20978
2019-07-28 13:40:09ganeshsetstatus: accepted -> needs-review
2019-07-28 19:51:23ganeshsetstatus: needs-review -> accepted
messages: + msg20996