4 ms·

Awesome, that's horrifying. One concrete issue with this technique: it doesn't "loop". Meaning, for example, if initially method #3 doesn't work but method #4

Awesome, that's horrifying. One concrete issue with this technique: it doesn't "loop". Meaning, for example, if initially method #3 doesn't work but method #4 does, it'll settle on method #4. If later, method #3 starts working, it won't switch it. Worse, if method #4 stops working, it'll just always fail. I imagine this isn't an issue for rsync's use but could be a concern if applying it more generally.

eridius 8y ago
According to the article, a step isn't skipped because it had a transient failure, it's skipped because it returns a special error code ENOSYS that means the syscall hasn't even been implemented, and therefore there's no point in ever trying it again.
- carlmr 8y ago
  That's correct. Although it's a very specific use-case.
- scatters 8y ago
  AIUI with containers it's now entirely possible that a running program may be migrated from a host where a syscall has not been implemented to one where it has.
vortico 8y ago
Just remove the `static` keyword and there you go. But it's an "optimization", and avoiding previously attempted methods is the whole point of the trick. Otherwise it would be an easy and uninteresting function.
- lazyjones 8y ago
  > Just remove the `static` keyword and there you go. That‘ll probably try all methods on every single file that is processed, so if method #16 is the one that works, you get 15 unnecessary syscalls for every file.
  - vortico 8y ago
    Exactly, that's what the parent comment was saying.
    - Sean1708 8y ago
      Lets say that there are only 3 methods and method #2 is the one that works initially. On the first call it'll try #1 and when that doesn't work it'll try #2 which will be successful. For the next however many calls it'll jump straight to #2. If #2 starts returning ENOSYS at some point it'll try #3 and if that works then all future calls will jump straight to #3. However if #3 stops working then it will keep on failing until #3 starts working again even if #1 started working after the first call. I think (I haven't actually tested it) you can do what the parent commenter wants by wrapping the whole thing in a do-loop but the original implementation is questionable enough, sticking an extra loop in there isn't doing anything to help make it less horrible. int set_the_mtime(...) { static int switch_step = 0; int loop_again = 1; do { switch (switch_step) { case 0: if (method_0_works(...)) return 0; switch_step++; /* FALLTHROUGH */ case 1: if (method_1_works(...)) return 0; switch_step++; /* FALLTHROUGH */ case 2: if (method_2_works(...)) return 0; } switch_step = 0; } while (loop_again--); return -1; /* ultimate failure */ }
kelnage 8y ago
My understanding of the code from the article is that if method #4 stopped working, it would just carry on down the list. The static variable just means where to skip to in the case list.

eridius 8y ago

According to the article, a step isn't skipped because it had a transient failure, it's skipped because it returns a special error code ENOSYS that means the syscall hasn't even been implemented, and therefore there's no point in ever trying it again.

carlmr 8y ago
That's correct. Although it's a very specific use-case.
scatters 8y ago
AIUI with containers it's now entirely possible that a running program may be migrated from a host where a syscall has not been implemented to one where it has.

vortico 8y ago

Just remove the `static` keyword and there you go. But it's an "optimization", and avoiding previously attempted methods is the whole point of the trick. Otherwise it would be an easy and uninteresting function.

lazyjones 8y ago
> Just remove the `static` keyword and there you go. That‘ll probably try all methods on every single file that is processed, so if method #16 is the one that works, you get 15 unnecessary syscalls for every file.
- vortico 8y ago
  Exactly, that's what the parent comment was saying.
  - Sean1708 8y ago
    Lets say that there are only 3 methods and method #2 is the one that works initially. On the first call it'll try #1 and when that doesn't work it'll try #2 which will be successful. For the next however many calls it'll jump straight to #2. If #2 starts returning ENOSYS at some point it'll try #3 and if that works then all future calls will jump straight to #3. However if #3 stops working then it will keep on failing until #3 starts working again even if #1 started working after the first call. I think (I haven't actually tested it) you can do what the parent commenter wants by wrapping the whole thing in a do-loop but the original implementation is questionable enough, sticking an extra loop in there isn't doing anything to help make it less horrible. int set_the_mtime(...) { static int switch_step = 0; int loop_again = 1; do { switch (switch_step) { case 0: if (method_0_works(...)) return 0; switch_step++; /* FALLTHROUGH */ case 1: if (method_1_works(...)) return 0; switch_step++; /* FALLTHROUGH */ case 2: if (method_2_works(...)) return 0; } switch_step = 0; } while (loop_again--); return -1; /* ultimate failure */ }

kelnage 8y ago

My understanding of the code from the article is that if method #4 stopped working, it would just carry on down the list. The static variable just means where to skip to in the case list.