-1

I wish to read/write an array (and update another one) with a parallelStream, without an index.

AtomicInteger doesn't allow bitwise operations, and using j.get() * 2 is slow:

final int[] j = {0};
ps.parallelStream().forEach(p -> {
    long k = next[j[0] << 1];                       
    for(; k < finalK; k += p)
        seg[(int) (k >>> S)] |= (1L << (k & BMASK));
    next[j[0] << 1] = (k - finalK);

    k = next[j[0] << 1 | 1];                        
    for(; k < finalK; k+= p) 
        seg[(int) (k >>> S)] |= (1L << (k & BMASK));
    next[j[0] << 1 | 1] = (k - finalK);
    j[0]++;
});

This is working in a sequential stream, not in parallel.

Sorry, if the seg[(int) (k >>> S)] |= (1L << (k & BMASK));seems unreadable, it's just the same as setting the bit k of a BitSet(seg is a long[]), wich is too much slow for my purpose.

Harry Cotte
  • 90
  • 2
  • 14
  • 3
    Indeed it doesn't. Did you have a question? You seem to be mutating your `j` array from multiple threads in an unsafe way. – Kayaman Feb 07 '20 at 09:14
  • Is the usage of `j[0]` a trick to avoid an error about `j` being not *final or effectlively final*? – MC Emperor Feb 07 '20 at 09:19
  • 4
    You're updating `next` here, based on the current value of `j`... Surely the order in which that's processed affects the result; so can you really do this in parallel? – Andy Turner Feb 07 '20 at 09:20
  • @AndyTurner `j[0] << 1 & 1` would be always zero, `j[0] << 1 | 1` is the same as `j[0] * 2 + 1` – Thomas Kläger Feb 07 '20 at 10:12
  • 1
    @ThomasKläger yeah, I realized that. It's a confusing way to write it, rather than just using multiplication and addition. – Andy Turner Feb 07 '20 at 10:13
  • 1
    @AndyTurner when using a C compiler from the eighties this might have been a usefull trick. Unfortunately these old tricks seem to live forever... – Thomas Kläger Feb 07 '20 at 10:24
  • 5
    First, I don’t understand the sentence “AtomicInteger doesnt allow bitwise operations on that index”. What is “that index” in the context of an AtomicInteger? If you mean the value of the AtomicInteger, of course, it *does* support bit operations. Not that it matters, as you are never performing an update using bit operations. The only update you are performing on `j[0]` is an ordinary increment. So there’s nothing in your code preventing to replace the single element array `j` with an `AtomicInteger`. Though this is unlikely to solve your problems, as your code depends on the processing order – Holger Feb 07 '20 at 11:13
  • 3
    Another thing worth reading: [What is the XY problem?](https://meta.stackexchange.com/questions/66377/what-is-the-xy-problem/66378#66378). You are asking for a fix for the fundamentally broken attempt to use an external counter, when the actual solution is not to use an external counter at all. This depends on what `ps` actually is. – Holger Feb 07 '20 at 11:36
  • 3
    @ThomasKläger but when you assume a non-optimizing compiler, you wouldn’t repeat `j[0] << 1` four times in a row. And, you’d possible consider that the action performed for the even and odd elements is identical, hence, this arithmetic is entirely obsolete… – Holger Feb 07 '20 at 11:53
  • @AndyTurner I think you are right, I cant do this in parallel. And using an `AtomicInteger` slow down.the loop by 10%. – Harry Cotte Feb 08 '20 at 08:43
  • 1
    "to avoid multiplication" you have far more fundamental issues in this code, namely correctness. Speed of incorrect code is irrelevant. By analogy: I'd rather take a slower taxi to the right place than a faster taxi that might drop me somewhere unexpected. (Plus, a shift and a multiplication by two will be as fast as each other). – Andy Turner Feb 08 '20 at 09:19
  • @AndyTurner I talk about the speed of the nonParallel loop. Btw I tested with an `AtomicLongArray`with an `AtomicInteger`as index, this doesnt change. So, I agree, this code is perfectly working but not in parallel, or I ve to refactor the algorithm. – Harry Cotte Feb 08 '20 at 11:08

1 Answers1

-1

Resolved using an ugly ConcurrentHashMap<Long, Long[]> wich avoid the index

next.replaceAll((p, k)->{

    for(; k[0] < finalK; k[0] += p)
        seg[(int) (k[0] >>> S)] |= (1L << (k[0] & BMASK));

    for(; k[1] < finalK; k[1] += p)                         
        seg[(int) (k[1] >>> S)] |= (1L << (k[1] & BMASK));

    return new long[]{k[0]-finalK, k[1]-finalK});
});

but this is very slow, and use much more memory

EDIT
I improved things using a little indexPairs class instead the array and a BitSet for clarity

ConcurrentHashMap<Long, indexPairs> next = initNext();
BitSet segment = new BitSet();
while(condition) {
    next.replaceAll((p, k) -> {
        for (; k.low < finalK; k.low += p)
            segment.set(k.low);
        k.low -= finalK;
        for (; k.high < finalK; k.high += p)                 
            segment.set(k.high);
        k.high -= finalK;
        return k;
    });
    ... Do something with segment
}
Harry Cotte
  • 90
  • 2
  • 14
  • 1
    There surely is a far more efficient solution possible, but you constantly fail to describe what you are actually trying to do. In your question’s code, you have a dependency to two different numbers, the `p` argument, originating from whatever `ps` is (for whatever reason you still refuse to tell us), and the number you tried to maintain via an external counter. In your map solution, these numbers seem to be the same (the map key). By the way, your map doesn’t “avoid the odd\even index”, in fact, *it’s the only thing you have kept* (zero/one in your first version, “low”/“high” in the second). – Holger Feb 10 '20 at 13:00
  • @Holger, as the `HashMap next` have a Long key, then `p` is a Long, and original `ps` is a List of Long. Sorry if I wasnt clear, and I never refuse help. (That's why I continue to search for a solution) And in my answer, I removed the use of the external counter – Harry Cotte Feb 10 '20 at 13:51