You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/11/17 13:25:49 UTC
[GitHub] [ignite] ivandasch opened a new pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
ivandasch opened a new pull request #9569:
URL: https://github.com/apache/ignite/pull/9569
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
### The Contribution Checklist
- [ ] There is a single JIRA ticket related to the pull request.
- [ ] The web-link to the pull request is attached to the JIRA ticket.
- [ ] The JIRA ticket has the _Patch Available_ state.
- [ ] The pull request body describes changes that have been made.
The description explains _WHAT_ and _WHY_ was made instead of _HOW_.
- [ ] The pull request title is treated as the final commit message.
The following pattern must be used: `IGNITE-XXXX Change summary` where `XXXX` - number of JIRA issue.
- [ ] A reviewer has been mentioned through the JIRA comments
(see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers))
- [ ] The pull request has been checked by the Teamcity Bot and
the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html))
### Notes
- [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
- [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
- [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
- [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r754333143
##########
File path: modules/numa-allocator/src/main/cpp/src/numa/numa_alloc.cpp
##########
@@ -0,0 +1,189 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <numa.h>
+#include <numa/numa_alloc.h>
+
+namespace numa {
+ class BitSet::BitSetImpl {
+ public:
+ BitSetImpl(): size(numa_max_node() + 1) {
+ mask = numa_bitmask_alloc(size);
+ }
+
+ BitSetImpl(const BitSetImpl& other): size(other.size) {
+ mask = numa_bitmask_alloc(size);
+ copy_bitmask_to_bitmask(other.mask, mask);
+ }
+
+ void SetBit(size_t idx, bool val) {
+ if (val)
+ numa_bitmask_setbit(mask, idx);
+ else
+ numa_bitmask_clearbit(mask, idx);
+ }
+
+ void SetAll(bool val) {
+ if (val)
+ numa_bitmask_setall(mask);
+ else
+ numa_bitmask_clearall(mask);
+ }
+
+ bool GetBit(size_t idx) const {
+ return numa_bitmask_isbitset(mask, idx);
+ }
+
+ bool Equals(const BitSetImpl& other) const {
+ return numa_bitmask_equal(mask, other.mask);
+ }
+
+ size_t Size() const {
+ return size;
+ }
+
+ bitmask* Raw() {
+ return mask;
+ }
+
+ ~BitSetImpl() {
+ numa_bitmask_free(mask);
+ }
+ private:
+ bitmask* mask;
+ size_t size;
+ };
+
+ BitSet::BitSet() {
+ pImpl = new BitSetImpl();
+ }
+
+ BitSet::BitSet(const BitSet &other) {
+ pImpl = new BitSetImpl(*other.pImpl);
+ }
+
+ BitSet& BitSet::operator=(const BitSet &other) {
+ if (this != &other) {
+ BitSet tmp(other);
+ std::swap(this->pImpl, tmp.pImpl);
+ }
+ return *this;
+ }
+
+ bool BitSet::Get(size_t pos) const {
+ return pImpl->GetBit(pos);
+ }
+
+ void BitSet::Set(size_t pos, bool value) {
+ pImpl->SetBit(pos, value);
+ }
+
+ void BitSet::Set() {
+ pImpl->SetAll(true);
+ }
+
+ void BitSet::Reset(size_t pos) {
+ pImpl->SetBit(pos, false);
+ }
+
+ void BitSet::Reset() {
+ pImpl->SetAll(false);
+ }
+
+ size_t BitSet::Size() const {
+ return pImpl->Size();
+ }
+
+ bool BitSet::operator==(const BitSet &other) {
+ return this->pImpl->Equals(*other.pImpl);
+ }
+
+ bool BitSet::operator!=(const BitSet &other) {
+ return !(*this == other);
+ }
+
+ BitSet::~BitSet() {
+ delete pImpl;
+ }
+
+ std::ostream &operator<<(std::ostream &os, const BitSet &set) {
+ os << '{';
+ for (size_t i = 0; i < set.Size(); ++i) {
+ os << set[i];
+ if (i < set.Size() - 1) {
+ os << ", ";
+ }
+ }
+ os << '}';
+ return os;
+ }
+
+ union region_size {
+ size_t size;
+ max_align_t a;
+ };
+
+ int NumaNodesCount() {
+ return numa_max_node() + 1;
Review comment:
Currently, numa_available just checks is the system call exists.
```
int numa_available(void)
{
if (get_mempolicy(NULL, NULL, 0, 0, 0) < 0 && errno == ENOSYS)
return -1;
return 0;
}
```
This syscall presents in kernel since 2.6.7. So on all kernels this will return 0;
If system is not NUMA system, it will have one numa node with index 0. Check it on your local linux installation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r754334395
##########
File path: modules/core/src/main/java/org/apache/ignite/mem/MemoryAllocator.java
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ignite.mem;
+
+/** */
+public interface MemoryAllocator {
+ /**
+ * @param size Size of allocated memory.
+ *
+ * @return Pointer to memory.
Review comment:
Yep, I am writing more verbose javadocs here and for others functions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r754342785
##########
File path: modules/numa-allocator/src/test/java/org/apache/ignite/internal/mem/NumaAllocatorBasicTest.java
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.mem;
+
+import java.util.Arrays;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.stream.IntStream;
+import java.util.stream.Stream;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.configuration.DataRegionConfiguration;
+import org.apache.ignite.configuration.DataStorageConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.persistence.DataRegion;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.mem.InterleavedNumaAllocationStrategy;
+import org.apache.ignite.mem.LocalNumaAllocationStrategy;
+import org.apache.ignite.mem.NumaAllocationStrategy;
+import org.apache.ignite.mem.NumaAllocator;
+import org.apache.ignite.mem.SimpleNumaAllocationStrategy;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class NumaAllocatorBasicTest extends GridCommonAbstractTest {
+ /** */
+ private static final long INITIAL_SIZE = 30L * 1024 * 1024;
+
+ /** */
+ private static final long MAX_SIZE = 100L * 1024 * 1024;
+
+ /** */
+ private static final String TEST_CACHE = "test";
+
+ /** */
+ private static final byte[] BUF = new byte[4096];
+
+ /** */
+ private static final int NUM_NODES = 3;
+
+ static {
+ ThreadLocalRandom.current().nextBytes(BUF);
+ }
+
+ /** */
+ @Parameterized.Parameters(name = "allocationStrategy={0}")
+ public static Iterable<Object[]> data() {
+ return Arrays.asList(
+ new Object[] {new LocalNumaAllocationStrategy()},
+ new Object[] {new InterleavedNumaAllocationStrategy(IntStream.range(0, NumaAllocUtil.NUMA_NODES_CNT)
+ .toArray())},
+ new Object[] {new SimpleNumaAllocationStrategy(NumaAllocUtil.NUMA_NODES_CNT - 1)}
+ );
+ }
+
+ /** */
+ @Parameterized.Parameter()
+ public NumaAllocationStrategy strategy;
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ DataStorageConfiguration memCfg = new DataStorageConfiguration()
+ .setDefaultDataRegionConfiguration(new DataRegionConfiguration()
+ .setInitialSize(INITIAL_SIZE)
+ .setMaxSize(MAX_SIZE)
+ .setMetricsEnabled(true)
+ .setMemoryAllocator(new NumaAllocator(strategy)));
+
+ cfg.setDataStorageConfiguration(memCfg);
+
+ return cfg;
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void beforeTest() throws Exception {
+ super.beforeTest();
+
+ startGrids(NUM_NODES);
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void afterTest() throws Exception {
+ super.afterTest();
+
+ stopAllGrids(true);
+ }
+
+ /** */
+ @Test
+ public void testLoadData() throws Exception {
+ IgniteEx client = startClientGrid("client");
+
+ client.getOrCreateCache(TEST_CACHE);
+
+ try (IgniteDataStreamer<Integer, byte[]> ds = client.dataStreamer(TEST_CACHE)) {
+ int cnt = 0;
+ while (hasFreeSpace()) {
+ ds.addData(++cnt, BUF);
+
+ if (cnt % 100 == 0)
+ ds.flush();
+ }
+ }
+
+ assertEquals(NUM_NODES, serverGrids().count());
+
+ serverGrids().forEach(g -> {
+ DataRegion dr = getDefaultRegion(g);
+
+ log().info("Allocated memory size on node " + g.name() + " :" + dr.metrics().getTotalAllocatedSize());
+
+ assertTrue(dr.metrics().getTotalAllocatedSize() > INITIAL_SIZE);
Review comment:
Nope, we checked not this in the loop.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r758179512
##########
File path: modules/numa-allocator/src/main/cpp/include/numa/numa_alloc.h
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef _NUMA_ALLOC_H
+#define _NUMA_ALLOC_H
+
+#include <ostream>
+
+namespace numa {
+ class BitSet {
+ public:
+ BitSet();
+
+ BitSet(const BitSet &other);
+
+ BitSet &operator=(const BitSet &other);
+
+ ~BitSet();
+
+ class Reference {
Review comment:
Now it is not used. This is quite common technique (see i.e. bitset implementation in [libstdc++|https://github.com/gcc-mirror/gcc/blob/85e91ad55a69282c1b0e34569836a026a1a954d1/libstdc%2B%2B-v3/include/std/bitset#L802]) to implement non-const `operator[]`. With it you can do somethin like this
```c++
BitSet a, b;
a[0] = true;
for (size_t i = 0; i < a.Size() && i < b.Size(); ++i) {
a[i] = !b[i];
}
```
But this functionality is no used so far, it is for probably future uses
##########
File path: modules/numa-allocator/src/main/java/org/apache/ignite/internal/mem/NumaAllocUtil.java
##########
@@ -0,0 +1,153 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.mem;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Paths;
+
+/** */
+public class NumaAllocUtil {
+ /** */
+ private static final String extension = ".so";
+
+ /** */
+ private static final String libraryName = "libnuma_alloc";
+
+ /** */
+ private NumaAllocUtil() {
+ // No-op.
+ }
+
+ /** */
+ private static String getLibName() {
+ return Paths.get("/org/apache/ignite/internal/mem", getOSName(), getOSArch(), libraryName + extension)
+ .toString();
+ }
+
+ /** */
+ private static String getOSArch() {
+ return System.getProperty("os.arch", "");
+ }
+
+ /** */
+ private static String getOSName() {
+ if (System.getProperty("os.name", "").contains("Linux"))
+ return "linux";
+ else
+ throw new UnsupportedOperationException("Operating System is not supported");
+ }
+
+ static {
+ String libName = getLibName();
+ File nativeLib = null;
+
+ try (InputStream in = NumaAllocUtil.class.getResourceAsStream(libName)) {
+ if (in == null) {
+ throw new ExceptionInInitializerError("Failed to load native numa_alloc library, "
+ + libName + " not found");
+ }
+
+ nativeLib = File.createTempFile(libraryName, extension);
+
+ try (FileOutputStream out = new FileOutputStream(nativeLib)) {
Review comment:
+1, good catch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] zstan commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
zstan commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r766615062
##########
File path: modules/numa-allocator/src/main/java/org/apache/ignite/internal/mem/NumaAllocUtil.java
##########
@@ -0,0 +1,151 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.mem;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Paths;
+
+/** */
+public class NumaAllocUtil {
+ /** */
+ private static final String extension = ".so";
+
+ /** */
+ private static final String libraryName = "libnuma_alloc";
+
+ /** */
+ private NumaAllocUtil() {
+ // No-op.
+ }
+
+ /** */
+ private static String getLibName() {
+ return Paths.get("/org/apache/ignite/internal/mem", getOSName(), getOSArch(), libraryName + extension)
+ .toString();
+ }
+
+ /** */
+ private static String getOSArch() {
Review comment:
who call this one?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r758182277
##########
File path: modules/numa-allocator/src/main/cpp/src/numa/numa_alloc.cpp
##########
@@ -0,0 +1,196 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <numa.h>
+#include <numa/numa_alloc.h>
+
+namespace numa {
+ class BitSet::BitSetImpl {
+ public:
+ BitSetImpl() : size_(numa_max_node() + 1) {
+ mask_ = numa_bitmask_alloc(size_);
+ }
+
+ BitSetImpl(const BitSetImpl &other) : size_(other.size_) {
+ mask_ = numa_bitmask_alloc(size_);
+ copy_bitmask_to_bitmask(other.mask_, mask_);
+ }
+
+ void SetBit(size_t idx, bool val) {
+ if (idx < size_) {
+ if (val)
+ numa_bitmask_setbit(mask_, idx);
+ else
+ numa_bitmask_clearbit(mask_, idx);
+ }
+ }
+
+ void SetAll(bool val) {
+ if (val)
+ numa_bitmask_setall(mask_);
+ else
+ numa_bitmask_clearall(mask_);
+ }
+
+ bool GetBit(size_t idx) const {
+ if (idx < size_)
+ return numa_bitmask_isbitset(mask_, idx);
+ else
+ return false;
+ }
+
+ bool Equals(const BitSetImpl &other) const {
+ return numa_bitmask_equal(mask_, other.mask_);
+ }
+
+ size_t Size() const {
+ return size_;
+ }
+
+ bitmask *Raw() {
+ return mask_;
+ }
+
+ ~BitSetImpl() {
+ numa_bitmask_free(mask_);
+ }
+
+ private:
+ bitmask *mask_;
+ size_t size_;
+ };
+
+ BitSet::BitSet() {
+ p_impl_ = new BitSetImpl();
+ }
+
+ BitSet::BitSet(const BitSet &other) {
+ p_impl_ = new BitSetImpl(*other.p_impl_);
+ }
+
+ BitSet &BitSet::operator=(const BitSet &other) {
+ if (this != &other) {
+ BitSet tmp(other);
+ std::swap(this->p_impl_, tmp.p_impl_);
+ }
+ return *this;
+ }
+
+ bool BitSet::Get(size_t pos) const {
+ return p_impl_->GetBit(pos);
+ }
+
+ void BitSet::Set(size_t pos, bool value) {
+ p_impl_->SetBit(pos, value);
+ }
+
+ void BitSet::Set() {
+ p_impl_->SetAll(true);
+ }
+
+ void BitSet::Reset(size_t pos) {
+ p_impl_->SetBit(pos, false);
+ }
+
+ void BitSet::Reset() {
+ p_impl_->SetAll(false);
+ }
+
+ size_t BitSet::Size() const {
+ return p_impl_->Size();
+ }
+
+ bool BitSet::operator==(const BitSet &other) {
+ return this->p_impl_->Equals(*other.p_impl_);
+ }
+
+ bool BitSet::operator!=(const BitSet &other) {
+ return !(*this == other);
+ }
+
+ BitSet::~BitSet() {
+ delete p_impl_;
+ }
+
+ std::ostream &operator<<(std::ostream &os, const BitSet &set) {
+ os << '{';
+ for (size_t i = 0; i < set.Size(); ++i) {
+ os << set[i];
+ if (i < set.Size() - 1) {
+ os << ", ";
+ }
+ }
+ os << '}';
+ return os;
+ }
+
+ union region_size {
+ size_t size;
+ max_align_t a;
+ };
+
+ int NumaNodesCount() {
+ return numa_max_node() + 1;
+ }
+
+ template<typename Func, typename ...Args>
+ void *NumaAllocHelper(Func f, size_t size, Args ...args) {
+ auto ptr = static_cast<region_size *>(f(size + sizeof(region_size), args...));
Review comment:
Yep,it should.
It is quite common trick to save size of allocated memory in first bytes of allocated region. But memory should be properly aligned, so I use union here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r763730827
##########
File path: modules/numa-allocator/src/main/java/org/apache/ignite/mem/NumaAllocator.java
##########
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.mem;
+
+import java.io.Serializable;
+import org.apache.ignite.internal.mem.NumaAllocUtil;
+
+/**
+ * NUMA aware memory allocator. Uses {@code libnuma} under the hood. Only Linux distros with {@code libnuma >= 2.0.x} is
Review comment:
Thanks for catching this typo
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r766624644
##########
File path: modules/numa-allocator/src/main/java/org/apache/ignite/internal/mem/NumaAllocUtil.java
##########
@@ -0,0 +1,151 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.mem;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Paths;
+
+/** */
+public class NumaAllocUtil {
+ /** */
+ private static final String extension = ".so";
+
+ /** */
+ private static final String libraryName = "libnuma_alloc";
+
+ /** */
+ private NumaAllocUtil() {
+ // No-op.
+ }
+
+ /** */
+ private static String getLibName() {
+ return Paths.get("/org/apache/ignite/internal/mem", getOSName(), getOSArch(), libraryName + extension)
+ .toString();
+ }
+
+ /** */
+ private static String getOSArch() {
Review comment:
```
/** */
private static String getLibName() {
return Paths.get("/org/apache/ignite/internal/mem", getOSName(), getOSArch(), libraryName + extension)
.toString();
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch closed pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch closed pull request #9569:
URL: https://github.com/apache/ignite/pull/9569
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r754340561
##########
File path: modules/numa-allocator/src/test/java/org/apache/ignite/internal/mem/NumaAllocatorUnitTest.java
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.mem;
+
+import java.util.Arrays;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.util.GridUnsafe;
+import org.apache.ignite.mem.InterleavedNumaAllocationStrategy;
+import org.apache.ignite.mem.LocalNumaAllocationStrategy;
+import org.apache.ignite.mem.NumaAllocationStrategy;
+import org.apache.ignite.mem.NumaAllocator;
+import org.apache.ignite.mem.SimpleNumaAllocationStrategy;
+import org.apache.ignite.testframework.GridTestUtils;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.experimental.runners.Enclosed;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Enclosed.class)
+public class NumaAllocatorUnitTest {
+ /** */
+ @RunWith(Parameterized.class)
+ public static class PositiveScenarioTest extends GridCommonAbstractTest {
+ /** */
+ private static final long BUF_SZ = 32 * 1024 * 1024;
Review comment:
It is not necessary here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] zstan commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
zstan commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r766621118
##########
File path: modules/numa-allocator/src/test/java/org/apache/ignite/internal/mem/NumaAllocatorBasicTest.java
##########
@@ -0,0 +1,175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.mem;
+
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import java.util.stream.Stream;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.configuration.DataRegionConfiguration;
+import org.apache.ignite.configuration.DataStorageConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.persistence.DataRegion;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.mem.InterleavedNumaAllocationStrategy;
+import org.apache.ignite.mem.LocalNumaAllocationStrategy;
+import org.apache.ignite.mem.NumaAllocationStrategy;
+import org.apache.ignite.mem.NumaAllocator;
+import org.apache.ignite.mem.SimpleNumaAllocationStrategy;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class NumaAllocatorBasicTest extends GridCommonAbstractTest {
+ /** */
+ private static final long INITIAL_SIZE = 30L * 1024 * 1024;
+
+ /** */
+ private static final long MAX_SIZE = 100L * 1024 * 1024;
+
+ /** */
+ private static final String TEST_CACHE = "test";
+
+ /** */
+ private static final byte[] BUF = new byte[4096];
+
+ /** */
+ private static final int NUM_NODES = 3;
+
+ static {
+ ThreadLocalRandom.current().nextBytes(BUF);
+ }
+
+ /** */
+ @Parameterized.Parameters(name = "allocationStrategy={0}, defaultConfig={1}")
+ public static Iterable<Object[]> data() {
+ return Stream.of(
+ new LocalNumaAllocationStrategy(),
+ new InterleavedNumaAllocationStrategy(IntStream.range(0, NumaAllocUtil.NUMA_NODES_CNT).toArray()),
Review comment:
append InterleavedNumaAllocationStrategy without predefined nodes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r758184269
##########
File path: modules/numa-allocator/pom.xml
##########
@@ -0,0 +1,178 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+<!--
+ POM file.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <parent>
+ <groupId>org.apache.ignite</groupId>
+ <artifactId>ignite-parent</artifactId>
+ <version>1</version>
+ <relativePath>../../parent</relativePath>
+ </parent>
+
+ <artifactId>ignite-numa-allocator</artifactId>
+ <version>${revision}</version>
+ <url>http://ignite.apache.org</url>
+
+ <dependencies>
+ <dependency>
+ <groupId>${project.groupId}</groupId>
+ <artifactId>ignite-core</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>${project.groupId}</groupId>
+ <artifactId>ignite-core</artifactId>
+ <version>${project.version}</version>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>${project.groupId}</groupId>
+ <artifactId>ignite-tools</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.springframework</groupId>
+ <artifactId>spring-beans</artifactId>
+ <version>${spring.version}</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.springframework</groupId>
+ <artifactId>spring-context</artifactId>
+ <version>${spring.version}</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>com.thoughtworks.xstream</groupId>
+ <artifactId>xstream</artifactId>
+ <version>1.4.8</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <version>${mockito.version}</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>log4j</groupId>
+ <artifactId>log4j</artifactId>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-antrun-plugin</artifactId>
Review comment:
I prefer for this task ant plugin because of
1. It is very stable and it is used in many projects for similar tasks.
2. It is explicit and steps are understandable.
3. I want to skip build in non-Linux systems.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] SammyVimes commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
SammyVimes commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r758123632
##########
File path: modules/numa-allocator/src/main/cpp/src/numa/numa_alloc.cpp
##########
@@ -0,0 +1,196 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <numa.h>
+#include <numa/numa_alloc.h>
+
+namespace numa {
+ class BitSet::BitSetImpl {
+ public:
+ BitSetImpl() : size_(numa_max_node() + 1) {
+ mask_ = numa_bitmask_alloc(size_);
+ }
+
+ BitSetImpl(const BitSetImpl &other) : size_(other.size_) {
+ mask_ = numa_bitmask_alloc(size_);
+ copy_bitmask_to_bitmask(other.mask_, mask_);
+ }
+
+ void SetBit(size_t idx, bool val) {
+ if (idx < size_) {
+ if (val)
+ numa_bitmask_setbit(mask_, idx);
+ else
+ numa_bitmask_clearbit(mask_, idx);
+ }
+ }
+
+ void SetAll(bool val) {
+ if (val)
+ numa_bitmask_setall(mask_);
+ else
+ numa_bitmask_clearall(mask_);
+ }
+
+ bool GetBit(size_t idx) const {
+ if (idx < size_)
+ return numa_bitmask_isbitset(mask_, idx);
+ else
+ return false;
+ }
+
+ bool Equals(const BitSetImpl &other) const {
+ return numa_bitmask_equal(mask_, other.mask_);
+ }
+
+ size_t Size() const {
+ return size_;
+ }
+
+ bitmask *Raw() {
+ return mask_;
+ }
+
+ ~BitSetImpl() {
+ numa_bitmask_free(mask_);
+ }
+
+ private:
+ bitmask *mask_;
+ size_t size_;
+ };
+
+ BitSet::BitSet() {
+ p_impl_ = new BitSetImpl();
+ }
+
+ BitSet::BitSet(const BitSet &other) {
+ p_impl_ = new BitSetImpl(*other.p_impl_);
+ }
+
+ BitSet &BitSet::operator=(const BitSet &other) {
+ if (this != &other) {
+ BitSet tmp(other);
+ std::swap(this->p_impl_, tmp.p_impl_);
+ }
+ return *this;
+ }
+
+ bool BitSet::Get(size_t pos) const {
+ return p_impl_->GetBit(pos);
+ }
+
+ void BitSet::Set(size_t pos, bool value) {
+ p_impl_->SetBit(pos, value);
+ }
+
+ void BitSet::Set() {
+ p_impl_->SetAll(true);
+ }
+
+ void BitSet::Reset(size_t pos) {
+ p_impl_->SetBit(pos, false);
+ }
+
+ void BitSet::Reset() {
+ p_impl_->SetAll(false);
+ }
+
+ size_t BitSet::Size() const {
+ return p_impl_->Size();
+ }
+
+ bool BitSet::operator==(const BitSet &other) {
+ return this->p_impl_->Equals(*other.p_impl_);
+ }
+
+ bool BitSet::operator!=(const BitSet &other) {
+ return !(*this == other);
+ }
+
+ BitSet::~BitSet() {
+ delete p_impl_;
+ }
+
+ std::ostream &operator<<(std::ostream &os, const BitSet &set) {
+ os << '{';
+ for (size_t i = 0; i < set.Size(); ++i) {
+ os << set[i];
+ if (i < set.Size() - 1) {
+ os << ", ";
+ }
+ }
+ os << '}';
+ return os;
+ }
+
+ union region_size {
+ size_t size;
+ max_align_t a;
+ };
+
+ int NumaNodesCount() {
+ return numa_max_node() + 1;
+ }
+
+ template<typename Func, typename ...Args>
+ void *NumaAllocHelper(Func f, size_t size, Args ...args) {
+ auto ptr = static_cast<region_size *>(f(size + sizeof(region_size), args...));
Review comment:
I think here should be a comment about the layout of a memory that is allocated. Something like "allocate structure + raw memory and return the pointer to raw memory" but in a nicer way
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r763730543
##########
File path: modules/numa-allocator/src/main/java/org/apache/ignite/mem/SimpleNumaAllocationStrategy.java
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.mem;
+
+import java.io.Serializable;
+import org.apache.ignite.internal.mem.NumaAllocUtil;
+import org.apache.ignite.internal.util.tostring.GridToStringBuilder;
+import org.apache.ignite.internal.util.tostring.GridToStringInclude;
+import org.apache.ignite.internal.util.typedef.internal.A;
+
+/**
+ * Simple NUMA allocation strategy.
+ * <p>
+ * Use {@link SimpleNumaAllocationStrategy#SimpleNumaAllocationStrategy(int)} to allocate memory on specific NUMA node
+ * with number equals to {@code node}. Memory will be allocated using {@code void *numa_alloc_onnode(size_t, int)}
+ * of {@code libnuma}.
+ * <p>
+ * Use {@link SimpleNumaAllocationStrategy#SimpleNumaAllocationStrategy()} to allocate memory using default NUMA
+ * memory policy of current thread. Memory will be allocated using {@code void *numa_alloc(size_t)} of {@code lubnuma}.
+ * Memory policy could be set by running application with {@code numactl}
+ */
+public class SimpleNumaAllocationStrategy implements NumaAllocationStrategy, Serializable {
+ /** */
+ private static final long serialVersionUID = 0L;
+
+ /** */
+ @GridToStringInclude
+ private final int node;
+
+ /** */
+ public SimpleNumaAllocationStrategy() {
+ node = Integer.MAX_VALUE;
Review comment:
Make sense, like `std::npos = size_t(-1)`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r766629061
##########
File path: modules/numa-allocator/src/test/java/org/apache/ignite/internal/mem/NumaAllocatorBasicTest.java
##########
@@ -0,0 +1,175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.mem;
+
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import java.util.stream.Stream;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.configuration.DataRegionConfiguration;
+import org.apache.ignite.configuration.DataStorageConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.persistence.DataRegion;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.mem.InterleavedNumaAllocationStrategy;
+import org.apache.ignite.mem.LocalNumaAllocationStrategy;
+import org.apache.ignite.mem.NumaAllocationStrategy;
+import org.apache.ignite.mem.NumaAllocator;
+import org.apache.ignite.mem.SimpleNumaAllocationStrategy;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class NumaAllocatorBasicTest extends GridCommonAbstractTest {
+ /** */
+ private static final long INITIAL_SIZE = 30L * 1024 * 1024;
+
+ /** */
+ private static final long MAX_SIZE = 100L * 1024 * 1024;
+
+ /** */
+ private static final String TEST_CACHE = "test";
+
+ /** */
+ private static final byte[] BUF = new byte[4096];
+
+ /** */
+ private static final int NUM_NODES = 3;
+
+ static {
+ ThreadLocalRandom.current().nextBytes(BUF);
+ }
+
+ /** */
+ @Parameterized.Parameters(name = "allocationStrategy={0}, defaultConfig={1}")
+ public static Iterable<Object[]> data() {
+ return Stream.of(
+ new LocalNumaAllocationStrategy(),
+ new InterleavedNumaAllocationStrategy(IntStream.range(0, NumaAllocUtil.NUMA_NODES_CNT).toArray()),
Review comment:
+1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r766623319
##########
File path: modules/numa-allocator/src/main/java/org/apache/ignite/mem/InterleavedNumaAllocationStrategy.java
##########
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.mem;
+
+import java.io.Serializable;
+import java.util.Arrays;
+import org.apache.ignite.internal.mem.NumaAllocUtil;
+import org.apache.ignite.internal.util.tostring.GridToStringBuilder;
+import org.apache.ignite.internal.util.tostring.GridToStringInclude;
+import org.apache.ignite.internal.util.typedef.internal.A;
+
+/**
+ * Interleaved NUMA allocation strategy.
+ * <p>
+ * Use {@link InterleavedNumaAllocationStrategy#InterleavedNumaAllocationStrategy()} to allocate memory interleaved
+ * on all available NUMA nodes. Memory will be allocated using {@code void *numa_alloc_interleaved(size_t)}
+ * of {@code libnuma}.
+ * <p>
+ * Use {@link InterleavedNumaAllocationStrategy#InterleavedNumaAllocationStrategy(int[])} to allocate memory interleaved
+ * on specified nodes.
+ * {@code void *numa_alloc_interleaved_subset(size_t, struct bitmask*)} of {@code lubnuma}.
+ */
+public class InterleavedNumaAllocationStrategy implements NumaAllocationStrategy, Serializable {
+ /** */
+ private static final long serialVersionUID = 0L;
+
+ /** */
+ @GridToStringInclude
+ private final int[] nodes;
+
+ /** */
+ public InterleavedNumaAllocationStrategy() {
+ this(null);
+ }
+
+ /**
+ * @param nodes Array of NUMA nodes to allocate on.
+ */
+ public InterleavedNumaAllocationStrategy(int[] nodes) {
+ if (nodes != null && nodes.length > 0) {
+ this.nodes = Arrays.copyOf(nodes, nodes.length);
+
+ Arrays.sort(this.nodes);
+ A.ensure(this.nodes[0] >= 0, "NUMA node number must be positive, passed instead "
+ + Arrays.toString(this.nodes));
+ A.ensure(this.nodes[this.nodes.length - 1] < NumaAllocUtil.NUMA_NODES_CNT,
Review comment:
Yep, they can.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] zstan commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
zstan commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r766613194
##########
File path: modules/numa-allocator/src/main/cpp/src/org_apache_ignite_internal_mem_NumaAllocUtil.cpp
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <numa/numa_alloc.h>
+#include "org_apache_ignite_internal_mem_NumaAllocUtil.h"
+
+class JIntArrayGuard {
+public:
+ JIntArrayGuard(JNIEnv* env, jintArray arr): env_(env), java_arr_(arr) {
+ arr_ = env->GetIntArrayElements(java_arr_, nullptr);
+ length_ = static_cast<size_t>(env->GetArrayLength(java_arr_));
+ }
+
+ JIntArrayGuard() = delete;
+ JIntArrayGuard(const JIntArrayGuard&) = delete;
+ JIntArrayGuard& operator=(const JIntArrayGuard&) = delete;
+
+ ~JIntArrayGuard() {
+ env_->ReleaseIntArrayElements(java_arr_, arr_, 0);
+ }
+
+ const int& operator[](size_t pos) const {
+ return arr_[pos];
+ }
+
+ int& operator[](size_t pos) {
+ return arr_[pos];
+ }
+
+ size_t Size() const {
+ return length_;
+ }
+private:
+ JNIEnv* env_;
+ jintArray java_arr_;
+ jint* arr_;
+ size_t length_;
+};
+
+JNIEXPORT jlong JNICALL Java_org_apache_ignite_internal_mem_NumaAllocUtil_allocate(
+ JNIEnv*,
+ jclass,
+ jlong size
+) {
+ void* ptr = numa::Alloc(static_cast<size_t>(size));
+ return reinterpret_cast<jlong>(ptr);
+}
+
+JNIEXPORT jlong JNICALL Java_org_apache_ignite_internal_mem_NumaAllocUtil_allocateOnNode(
+ JNIEnv*,
+ jclass,
+ jlong size,
+ jint node
+) {
+ void* ptr;
+ auto size_ = static_cast<size_t>(size);
+ if (node >= 0 && node < numa::NumaNodesCount()) {
+ ptr = numa::Alloc(size_, static_cast<int>(node));
+ }
+ else {
+ ptr = numa::Alloc(size_);
+ }
+ return reinterpret_cast<jlong>(ptr);
+}
+
+JNIEXPORT jlong JNICALL Java_org_apache_ignite_internal_mem_NumaAllocUtil_allocateLocal(
+ JNIEnv*,
+ jclass,
+ jlong size
+) {
+ void* ptr = numa::AllocLocal(static_cast<size_t>(size));
+ return reinterpret_cast<jlong>(ptr);
+}
+
+JNIEXPORT jlong JNICALL Java_org_apache_ignite_internal_mem_NumaAllocUtil_allocateInterleaved(
+ JNIEnv *jniEnv,
+ jclass,
+ jlong size,
+ jintArray arr
+) {
+ void* ptr;
+ auto size_ = static_cast<size_t>(size);
+ if (arr != nullptr) {
+ JIntArrayGuard nodes(jniEnv, arr);
Review comment:
possibly it`s more lightweight to call jsize len = (*env)->GetArrayLength(env, arr); and if size > 0 construct nodes ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r758272183
##########
File path: modules/numa-allocator/src/main/java/org/apache/ignite/internal/mem/NumaAllocUtil.java
##########
@@ -0,0 +1,153 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.mem;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Paths;
+
+/** */
+public class NumaAllocUtil {
+ /** */
+ private static final String extension = ".so";
+
+ /** */
+ private static final String libraryName = "libnuma_alloc";
+
+ /** */
+ private NumaAllocUtil() {
+ // No-op.
+ }
+
+ /** */
+ private static String getLibName() {
+ return Paths.get("/org/apache/ignite/internal/mem", getOSName(), getOSArch(), libraryName + extension)
+ .toString();
+ }
+
+ /** */
+ private static String getOSArch() {
+ return System.getProperty("os.arch", "");
+ }
+
+ /** */
+ private static String getOSName() {
+ if (System.getProperty("os.name", "").contains("Linux"))
+ return "linux";
+ else
+ throw new UnsupportedOperationException("Operating System is not supported");
+ }
+
+ static {
+ String libName = getLibName();
+ File nativeLib = null;
+
+ try (InputStream in = NumaAllocUtil.class.getResourceAsStream(libName)) {
+ if (in == null) {
+ throw new ExceptionInInitializerError("Failed to load native numa_alloc library, "
+ + libName + " not found");
+ }
+
+ nativeLib = File.createTempFile(libraryName, extension);
+
+ try (FileOutputStream out = new FileOutputStream(nativeLib)) {
Review comment:
Unfortunately, this is not suitable. Suitable method (`Files.copy(InputStream,OutputStream)` is private,
others method implies recreation of file. Also, there is not any good method to create temp file name.
So I will not change this implementation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r766626104
##########
File path: modules/numa-allocator/src/main/cpp/src/org_apache_ignite_internal_mem_NumaAllocUtil.cpp
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <numa/numa_alloc.h>
+#include "org_apache_ignite_internal_mem_NumaAllocUtil.h"
+
+class JIntArrayGuard {
+public:
+ JIntArrayGuard(JNIEnv* env, jintArray arr): env_(env), java_arr_(arr) {
+ arr_ = env->GetIntArrayElements(java_arr_, nullptr);
+ length_ = static_cast<size_t>(env->GetArrayLength(java_arr_));
+ }
+
+ JIntArrayGuard() = delete;
+ JIntArrayGuard(const JIntArrayGuard&) = delete;
+ JIntArrayGuard& operator=(const JIntArrayGuard&) = delete;
+
+ ~JIntArrayGuard() {
+ env_->ReleaseIntArrayElements(java_arr_, arr_, 0);
+ }
+
+ const int& operator[](size_t pos) const {
+ return arr_[pos];
+ }
+
+ int& operator[](size_t pos) {
+ return arr_[pos];
+ }
+
+ size_t Size() const {
+ return length_;
+ }
+private:
+ JNIEnv* env_;
+ jintArray java_arr_;
+ jint* arr_;
+ size_t length_;
+};
+
+JNIEXPORT jlong JNICALL Java_org_apache_ignite_internal_mem_NumaAllocUtil_allocate(
+ JNIEnv*,
+ jclass,
+ jlong size
+) {
+ void* ptr = numa::Alloc(static_cast<size_t>(size));
+ return reinterpret_cast<jlong>(ptr);
+}
+
+JNIEXPORT jlong JNICALL Java_org_apache_ignite_internal_mem_NumaAllocUtil_allocateOnNode(
+ JNIEnv*,
+ jclass,
+ jlong size,
+ jint node
+) {
+ void* ptr;
+ auto size_ = static_cast<size_t>(size);
+ if (node >= 0 && node < numa::NumaNodesCount()) {
+ ptr = numa::Alloc(size_, static_cast<int>(node));
+ }
+ else {
+ ptr = numa::Alloc(size_);
+ }
+ return reinterpret_cast<jlong>(ptr);
+}
+
+JNIEXPORT jlong JNICALL Java_org_apache_ignite_internal_mem_NumaAllocUtil_allocateLocal(
+ JNIEnv*,
+ jclass,
+ jlong size
+) {
+ void* ptr = numa::AllocLocal(static_cast<size_t>(size));
+ return reinterpret_cast<jlong>(ptr);
+}
+
+JNIEXPORT jlong JNICALL Java_org_apache_ignite_internal_mem_NumaAllocUtil_allocateInterleaved(
+ JNIEnv *jniEnv,
+ jclass,
+ jlong size,
+ jintArray arr
+) {
+ void* ptr;
+ auto size_ = static_cast<size_t>(size);
+ if (arr != nullptr) {
+ JIntArrayGuard nodes(jniEnv, arr);
Review comment:
Creating object on stack is pretty lightweight.
Nevertheless, this call is not called quite often.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r766625626
##########
File path: modules/numa-allocator/src/main/java/org/apache/ignite/internal/mem/NumaAllocUtil.java
##########
@@ -0,0 +1,151 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.mem;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Paths;
+
+/** */
+public class NumaAllocUtil {
+ /** */
+ private static final String extension = ".so";
+
+ /** */
+ private static final String libraryName = "libnuma_alloc";
+
+ /** */
+ private NumaAllocUtil() {
+ // No-op.
+ }
+
+ /** */
+ private static String getLibName() {
+ return Paths.get("/org/apache/ignite/internal/mem", getOSName(), getOSArch(), libraryName + extension)
+ .toString();
+ }
+
+ /** */
+ private static String getOSArch() {
+ return System.getProperty("os.arch", "");
+ }
+
+ /** */
+ private static String getOSName() {
Review comment:
Yep it is correct, quite common technique -- http://lopica.sourceforge.net/os.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] rpuch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
rpuch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r763709643
##########
File path: modules/numa-allocator/src/main/java/org/apache/ignite/mem/NumaAllocator.java
##########
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.mem;
+
+import java.io.Serializable;
+import org.apache.ignite.internal.mem.NumaAllocUtil;
+
+/**
+ * NUMA aware memory allocator. Uses {@code libnuma} under the hood. Only Linux distros with {@code libnuma >= 2.0.x} is
Review comment:
'are supported'?
##########
File path: modules/numa-allocator/src/main/java/org/apache/ignite/mem/SimpleNumaAllocationStrategy.java
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.mem;
+
+import java.io.Serializable;
+import org.apache.ignite.internal.mem.NumaAllocUtil;
+import org.apache.ignite.internal.util.tostring.GridToStringBuilder;
+import org.apache.ignite.internal.util.tostring.GridToStringInclude;
+import org.apache.ignite.internal.util.typedef.internal.A;
+
+/**
+ * Simple NUMA allocation strategy.
+ * <p>
+ * Use {@link SimpleNumaAllocationStrategy#SimpleNumaAllocationStrategy(int)} to allocate memory on specific NUMA node
+ * with number equals to {@code node}. Memory will be allocated using {@code void *numa_alloc_onnode(size_t, int)}
+ * of {@code libnuma}.
+ * <p>
+ * Use {@link SimpleNumaAllocationStrategy#SimpleNumaAllocationStrategy()} to allocate memory using default NUMA
+ * memory policy of current thread. Memory will be allocated using {@code void *numa_alloc(size_t)} of {@code lubnuma}.
+ * Memory policy could be set by running application with {@code numactl}
+ */
+public class SimpleNumaAllocationStrategy implements NumaAllocationStrategy, Serializable {
+ /** */
+ private static final long serialVersionUID = 0L;
+
+ /** */
+ @GridToStringInclude
+ private final int node;
+
+ /** */
+ public SimpleNumaAllocationStrategy() {
+ node = Integer.MAX_VALUE;
Review comment:
A little suggestion: we might introduce a constant like `NOT_SPECIFIED` with the value of `Integer.MAX_VALUE` and use it on lines 47 and 64, this would make it a little bit more obvious what this special value means.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] zstan commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
zstan commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r766619268
##########
File path: modules/numa-allocator/src/main/java/org/apache/ignite/mem/InterleavedNumaAllocationStrategy.java
##########
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.mem;
+
+import java.io.Serializable;
+import java.util.Arrays;
+import org.apache.ignite.internal.mem.NumaAllocUtil;
+import org.apache.ignite.internal.util.tostring.GridToStringBuilder;
+import org.apache.ignite.internal.util.tostring.GridToStringInclude;
+import org.apache.ignite.internal.util.typedef.internal.A;
+
+/**
+ * Interleaved NUMA allocation strategy.
+ * <p>
+ * Use {@link InterleavedNumaAllocationStrategy#InterleavedNumaAllocationStrategy()} to allocate memory interleaved
+ * on all available NUMA nodes. Memory will be allocated using {@code void *numa_alloc_interleaved(size_t)}
+ * of {@code libnuma}.
+ * <p>
+ * Use {@link InterleavedNumaAllocationStrategy#InterleavedNumaAllocationStrategy(int[])} to allocate memory interleaved
+ * on specified nodes.
+ * {@code void *numa_alloc_interleaved_subset(size_t, struct bitmask*)} of {@code lubnuma}.
+ */
+public class InterleavedNumaAllocationStrategy implements NumaAllocationStrategy, Serializable {
+ /** */
+ private static final long serialVersionUID = 0L;
+
+ /** */
+ @GridToStringInclude
+ private final int[] nodes;
+
+ /** */
+ public InterleavedNumaAllocationStrategy() {
+ this(null);
+ }
+
+ /**
+ * @param nodes Array of NUMA nodes to allocate on.
+ */
+ public InterleavedNumaAllocationStrategy(int[] nodes) {
+ if (nodes != null && nodes.length > 0) {
+ this.nodes = Arrays.copyOf(nodes, nodes.length);
+
+ Arrays.sort(this.nodes);
+ A.ensure(this.nodes[0] >= 0, "NUMA node number must be positive, passed instead "
+ + Arrays.toString(this.nodes));
+ A.ensure(this.nodes[this.nodes.length - 1] < NumaAllocUtil.NUMA_NODES_CNT,
Review comment:
this.nodes[this.nodes.length - 1] and only last ? can nodes can be like nodes = [0, 2, 3] ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] zstan commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
zstan commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r766618812
##########
File path: modules/numa-allocator/src/main/java/org/apache/ignite/mem/InterleavedNumaAllocationStrategy.java
##########
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.mem;
+
+import java.io.Serializable;
+import java.util.Arrays;
+import org.apache.ignite.internal.mem.NumaAllocUtil;
+import org.apache.ignite.internal.util.tostring.GridToStringBuilder;
+import org.apache.ignite.internal.util.tostring.GridToStringInclude;
+import org.apache.ignite.internal.util.typedef.internal.A;
+
+/**
+ * Interleaved NUMA allocation strategy.
+ * <p>
+ * Use {@link InterleavedNumaAllocationStrategy#InterleavedNumaAllocationStrategy()} to allocate memory interleaved
+ * on all available NUMA nodes. Memory will be allocated using {@code void *numa_alloc_interleaved(size_t)}
+ * of {@code libnuma}.
+ * <p>
+ * Use {@link InterleavedNumaAllocationStrategy#InterleavedNumaAllocationStrategy(int[])} to allocate memory interleaved
+ * on specified nodes.
+ * {@code void *numa_alloc_interleaved_subset(size_t, struct bitmask*)} of {@code lubnuma}.
+ */
+public class InterleavedNumaAllocationStrategy implements NumaAllocationStrategy, Serializable {
+ /** */
+ private static final long serialVersionUID = 0L;
+
+ /** */
+ @GridToStringInclude
+ private final int[] nodes;
+
+ /** */
+ public InterleavedNumaAllocationStrategy() {
+ this(null);
+ }
+
+ /**
+ * @param nodes Array of NUMA nodes to allocate on.
+ */
+ public InterleavedNumaAllocationStrategy(int[] nodes) {
+ if (nodes != null && nodes.length > 0) {
+ this.nodes = Arrays.copyOf(nodes, nodes.length);
+
+ Arrays.sort(this.nodes);
+ A.ensure(this.nodes[0] >= 0, "NUMA node number must be positive, passed instead "
Review comment:
you check only 0 node, why ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r751294793
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/mem/unsafe/UnsafeMemoryProvider.java
##########
@@ -75,7 +79,7 @@ public UnsafeMemoryProvider(IgniteLogger log) {
DirectMemoryRegion chunk = it.next();
if (deallocate) {
Review comment:
Looks like it's possible to skip iteration over regions in case `deallocaiton is False`.
##########
File path: modules/numa-allocator/src/main/cpp/CMakeLists.txt
##########
@@ -0,0 +1,46 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+cmake_minimum_required(VERSION 3.6)
+project(numa_alloc)
+
+set (CMAKE_CXX_STANDARD 11)
Review comment:
rm whitespace
##########
File path: modules/numa-allocator/src/main/java/org/apache/ignite/mem/NumaAllocator.java
##########
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.mem;
+
+import java.io.Serializable;
+import org.apache.ignite.internal.mem.NumaAllocUtil;
+
+/** */
+public class NumaAllocator implements MemoryAllocator, Serializable {
Review comment:
Why does it specify the Serializable interface?
##########
File path: modules/numa-allocator/src/test/java/org/apache/ignite/internal/mem/NumaAllocatorUnitTest.java
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.mem;
+
+import java.util.Arrays;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.util.GridUnsafe;
+import org.apache.ignite.mem.InterleavedNumaAllocationStrategy;
+import org.apache.ignite.mem.LocalNumaAllocationStrategy;
+import org.apache.ignite.mem.NumaAllocationStrategy;
+import org.apache.ignite.mem.NumaAllocator;
+import org.apache.ignite.mem.SimpleNumaAllocationStrategy;
+import org.apache.ignite.testframework.GridTestUtils;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.experimental.runners.Enclosed;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Enclosed.class)
+public class NumaAllocatorUnitTest {
+ /** */
+ @RunWith(Parameterized.class)
+ public static class PositiveScenarioTest extends GridCommonAbstractTest {
+ /** */
+ private static final long BUF_SZ = 32 * 1024 * 1024;
+
+ /** */
+ private static final int[] EVEN_NODES = IntStream.range(0, NumaAllocUtil.NUMA_NODES_CNT)
+ .filter(x -> x % 2 == 0).toArray();
+
+ /** */
+ private static final int[] ALL_NODES = IntStream.range(0, NumaAllocUtil.NUMA_NODES_CNT).toArray();
+
+ /**
+ *
+ */
+ @Parameterized.Parameters(name = "allocationStrategy={0}")
+ public static Iterable<Object[]> data() {
+ return Arrays.asList(
+ new Object[] {new LocalNumaAllocationStrategy()},
+ new Object[] {new InterleavedNumaAllocationStrategy()},
+ new Object[] {new InterleavedNumaAllocationStrategy(new int[0])},
+ new Object[] {new InterleavedNumaAllocationStrategy(EVEN_NODES)},
+ new Object[] {new InterleavedNumaAllocationStrategy(ALL_NODES)},
+ new Object[] {new SimpleNumaAllocationStrategy()},
+ new Object[] {new SimpleNumaAllocationStrategy(NumaAllocUtil.NUMA_NODES_CNT - 1)}
+ );
+ }
+
+ /** */
+ @Parameterized.Parameter()
+ public NumaAllocationStrategy strategy;
+
+ /** */
+ @Test
+ public void test() {
+ NumaAllocator allocator = new NumaAllocator(strategy);
+
+ long ptr = 0;
+ try {
+ ptr = allocator.allocateMemory(BUF_SZ);
+
+ assertEquals(BUF_SZ, NumaAllocUtil.chunkSize(ptr));
+
+ GridUnsafe.setMemory(ptr, BUF_SZ, (byte)1);
+
+ for (long i = 0; i < BUF_SZ; i++) {
+ assertEquals((byte)1, GridUnsafe.getByte(ptr + i));
Review comment:
According to Ignite code inspections, oneliners should not be wrapped with curly braces.
##########
File path: modules/numa-allocator/src/test/java/org/apache/ignite/internal/mem/NumaAllocatorBasicTest.java
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.mem;
+
+import java.util.Arrays;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.stream.IntStream;
+import java.util.stream.Stream;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteDataStreamer;
+import org.apache.ignite.configuration.DataRegionConfiguration;
+import org.apache.ignite.configuration.DataStorageConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.cache.persistence.DataRegion;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.mem.InterleavedNumaAllocationStrategy;
+import org.apache.ignite.mem.LocalNumaAllocationStrategy;
+import org.apache.ignite.mem.NumaAllocationStrategy;
+import org.apache.ignite.mem.NumaAllocator;
+import org.apache.ignite.mem.SimpleNumaAllocationStrategy;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Parameterized.class)
+public class NumaAllocatorBasicTest extends GridCommonAbstractTest {
+ /** */
+ private static final long INITIAL_SIZE = 30L * 1024 * 1024;
+
+ /** */
+ private static final long MAX_SIZE = 100L * 1024 * 1024;
+
+ /** */
+ private static final String TEST_CACHE = "test";
+
+ /** */
+ private static final byte[] BUF = new byte[4096];
+
+ /** */
+ private static final int NUM_NODES = 3;
+
+ static {
+ ThreadLocalRandom.current().nextBytes(BUF);
+ }
+
+ /** */
+ @Parameterized.Parameters(name = "allocationStrategy={0}")
+ public static Iterable<Object[]> data() {
+ return Arrays.asList(
+ new Object[] {new LocalNumaAllocationStrategy()},
+ new Object[] {new InterleavedNumaAllocationStrategy(IntStream.range(0, NumaAllocUtil.NUMA_NODES_CNT)
+ .toArray())},
+ new Object[] {new SimpleNumaAllocationStrategy(NumaAllocUtil.NUMA_NODES_CNT - 1)}
+ );
+ }
+
+ /** */
+ @Parameterized.Parameter()
+ public NumaAllocationStrategy strategy;
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ DataStorageConfiguration memCfg = new DataStorageConfiguration()
+ .setDefaultDataRegionConfiguration(new DataRegionConfiguration()
+ .setInitialSize(INITIAL_SIZE)
+ .setMaxSize(MAX_SIZE)
+ .setMetricsEnabled(true)
+ .setMemoryAllocator(new NumaAllocator(strategy)));
+
+ cfg.setDataStorageConfiguration(memCfg);
+
+ return cfg;
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void beforeTest() throws Exception {
+ super.beforeTest();
+
+ startGrids(NUM_NODES);
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void afterTest() throws Exception {
+ super.afterTest();
+
+ stopAllGrids(true);
+ }
+
+ /** */
+ @Test
+ public void testLoadData() throws Exception {
+ IgniteEx client = startClientGrid("client");
+
+ client.getOrCreateCache(TEST_CACHE);
+
+ try (IgniteDataStreamer<Integer, byte[]> ds = client.dataStreamer(TEST_CACHE)) {
+ int cnt = 0;
+ while (hasFreeSpace()) {
+ ds.addData(++cnt, BUF);
+
+ if (cnt % 100 == 0)
+ ds.flush();
+ }
+ }
+
+ assertEquals(NUM_NODES, serverGrids().count());
+
+ serverGrids().forEach(g -> {
+ DataRegion dr = getDefaultRegion(g);
+
+ log().info("Allocated memory size on node " + g.name() + " :" + dr.metrics().getTotalAllocatedSize());
+
+ assertTrue(dr.metrics().getTotalAllocatedSize() > INITIAL_SIZE);
Review comment:
This will be always `true` because we already check this inside `hasFreeSpace`. I'd suggest to add an Assert in the while loop that the amount of allocated memory was changed after flushing data stream.
##########
File path: modules/numa-allocator/src/test/java/org/apache/ignite/internal/mem/NumaAllocatorUnitTest.java
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.mem;
+
+import java.util.Arrays;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.util.GridUnsafe;
+import org.apache.ignite.mem.InterleavedNumaAllocationStrategy;
+import org.apache.ignite.mem.LocalNumaAllocationStrategy;
+import org.apache.ignite.mem.NumaAllocationStrategy;
+import org.apache.ignite.mem.NumaAllocator;
+import org.apache.ignite.mem.SimpleNumaAllocationStrategy;
+import org.apache.ignite.testframework.GridTestUtils;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+import org.junit.experimental.runners.Enclosed;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/** */
+@RunWith(Enclosed.class)
+public class NumaAllocatorUnitTest {
+ /** */
+ @RunWith(Parameterized.class)
+ public static class PositiveScenarioTest extends GridCommonAbstractTest {
+ /** */
+ private static final long BUF_SZ = 32 * 1024 * 1024;
Review comment:
32L
##########
File path: modules/numa-allocator/src/main/cpp/src/numa/numa_alloc.cpp
##########
@@ -0,0 +1,189 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <numa.h>
+#include <numa/numa_alloc.h>
+
+namespace numa {
+ class BitSet::BitSetImpl {
+ public:
+ BitSetImpl(): size(numa_max_node() + 1) {
+ mask = numa_bitmask_alloc(size);
+ }
+
+ BitSetImpl(const BitSetImpl& other): size(other.size) {
+ mask = numa_bitmask_alloc(size);
+ copy_bitmask_to_bitmask(other.mask, mask);
+ }
+
+ void SetBit(size_t idx, bool val) {
+ if (val)
+ numa_bitmask_setbit(mask, idx);
+ else
+ numa_bitmask_clearbit(mask, idx);
+ }
+
+ void SetAll(bool val) {
+ if (val)
+ numa_bitmask_setall(mask);
+ else
+ numa_bitmask_clearall(mask);
+ }
+
+ bool GetBit(size_t idx) const {
+ return numa_bitmask_isbitset(mask, idx);
+ }
+
+ bool Equals(const BitSetImpl& other) const {
+ return numa_bitmask_equal(mask, other.mask);
+ }
+
+ size_t Size() const {
+ return size;
+ }
+
+ bitmask* Raw() {
+ return mask;
+ }
+
+ ~BitSetImpl() {
+ numa_bitmask_free(mask);
+ }
+ private:
+ bitmask* mask;
+ size_t size;
+ };
+
+ BitSet::BitSet() {
+ pImpl = new BitSetImpl();
+ }
+
+ BitSet::BitSet(const BitSet &other) {
+ pImpl = new BitSetImpl(*other.pImpl);
+ }
+
+ BitSet& BitSet::operator=(const BitSet &other) {
+ if (this != &other) {
+ BitSet tmp(other);
+ std::swap(this->pImpl, tmp.pImpl);
+ }
+ return *this;
+ }
+
+ bool BitSet::Get(size_t pos) const {
+ return pImpl->GetBit(pos);
+ }
+
+ void BitSet::Set(size_t pos, bool value) {
+ pImpl->SetBit(pos, value);
+ }
+
+ void BitSet::Set() {
+ pImpl->SetAll(true);
+ }
+
+ void BitSet::Reset(size_t pos) {
+ pImpl->SetBit(pos, false);
+ }
+
+ void BitSet::Reset() {
+ pImpl->SetAll(false);
+ }
+
+ size_t BitSet::Size() const {
+ return pImpl->Size();
+ }
+
+ bool BitSet::operator==(const BitSet &other) {
+ return this->pImpl->Equals(*other.pImpl);
+ }
+
+ bool BitSet::operator!=(const BitSet &other) {
+ return !(*this == other);
+ }
+
+ BitSet::~BitSet() {
+ delete pImpl;
+ }
+
+ std::ostream &operator<<(std::ostream &os, const BitSet &set) {
+ os << '{';
+ for (size_t i = 0; i < set.Size(); ++i) {
+ os << set[i];
+ if (i < set.Size() - 1) {
+ os << ", ";
+ }
+ }
+ os << '}';
+ return os;
+ }
+
+ union region_size {
+ size_t size;
+ max_align_t a;
+ };
+
+ int NumaNodesCount() {
+ return numa_max_node() + 1;
Review comment:
Should we add here, or somewhere a call `numa_available()` for ensure that it is possible to use NumaAllocator at all? Maybe we should add a such assertion in a constructor of the allocator java instance? WDYT?
##########
File path: modules/core/src/main/java/org/apache/ignite/mem/MemoryAllocator.java
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ignite.mem;
+
+/** */
+public interface MemoryAllocator {
+ /**
+ * @param size Size of allocated memory.
+ *
+ * @return Pointer to memory.
+ */
+ long allocateMemory(long size);
Review comment:
Ignite's code inspections require to mark all methods in interfaces with `public`.
##########
File path: modules/numa-allocator/src/test/resources/example-ignite.xml
##########
@@ -0,0 +1,79 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+<!--
+ Ignite configuration with all defaults and enabled p2p deployment and enabled events.
Review comment:
Comment mismatches the content
##########
File path: modules/core/src/main/java/org/apache/ignite/configuration/DataRegionConfiguration.java
##########
@@ -152,6 +153,9 @@
/** Warm-up configuration. */
@Nullable private WarmUpConfiguration warmUpCfg;
+ /** Memory allocator. */
Review comment:
Let's add a mention about default behavior.
##########
File path: modules/numa-allocator/src/main/java/org/apache/ignite/mem/LocalNumaAllocationStrategy.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.mem;
+
+import java.io.Serializable;
+import org.apache.ignite.internal.mem.NumaAllocUtil;
+import org.apache.ignite.internal.util.tostring.GridToStringBuilder;
+
+/** */
+public class LocalNumaAllocationStrategy implements NumaAllocationStrategy, Serializable {
Review comment:
Why does it specify the Serializable interface?
##########
File path: modules/numa-allocator/src/test/resources/example-ignite.xml
##########
@@ -0,0 +1,79 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+<!--
+ Ignite configuration with all defaults and enabled p2p deployment and enabled events.
+-->
+<beans xmlns="http://www.springframework.org/schema/beans"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://www.springframework.org/schema/beans
+ http://www.springframework.org/schema/beans/spring-beans.xsd">
+ <bean class="org.apache.ignite.configuration.IgniteConfiguration">
+ <property name="dataStorageConfiguration">
+ <bean class="org.apache.ignite.configuration.DataStorageConfiguration">
+ <property name="defaultDataRegionConfiguration">
+ <bean class="org.apache.ignite.configuration.DataRegionConfiguration">
+ <property name="name" value="Default_Region"/>
+ <!-- 100 MB memory region with disabled eviction -->
Review comment:
100MB --> 2GB
##########
File path: modules/core/src/main/java/org/apache/ignite/mem/MemoryAllocator.java
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ignite.mem;
+
+/** */
+public interface MemoryAllocator {
+ /**
+ * @param size Size of allocated memory.
+ *
+ * @return Pointer to memory.
Review comment:
WDYT, should this desc contain a short comment that method can return 0 in case underlying methods fails?
##########
File path: modules/numa-allocator/src/main/java/org/apache/ignite/internal/mem/NumaAllocUtil.java
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.mem;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+
+/** */
+public class NumaAllocUtil {
+ /** */
+ private static final String extension = ".so";
Review comment:
Should we add a var for the "libnuma_alloc" String too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r758179512
##########
File path: modules/numa-allocator/src/main/cpp/include/numa/numa_alloc.h
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef _NUMA_ALLOC_H
+#define _NUMA_ALLOC_H
+
+#include <ostream>
+
+namespace numa {
+ class BitSet {
+ public:
+ BitSet();
+
+ BitSet(const BitSet &other);
+
+ BitSet &operator=(const BitSet &other);
+
+ ~BitSet();
+
+ class Reference {
Review comment:
Now it is not used. This is quite common technique (see i.e. bitset implementation in [libstdc++](https://github.com/gcc-mirror/gcc/blob/85e91ad55a69282c1b0e34569836a026a1a954d1/libstdc%2B%2B-v3/include/std/bitset#L802)) to implement non-const `operator[]`. With it you can do somethin like this
```c++
BitSet a, b;
a[0] = true;
for (size_t i = 0; i < a.Size() && i < b.Size(); ++i) {
a[i] = !b[i];
}
```
But this functionality is not used so far, it is for probably future uses
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r754325699
##########
File path: modules/numa-allocator/src/main/java/org/apache/ignite/mem/LocalNumaAllocationStrategy.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.mem;
+
+import java.io.Serializable;
+import org.apache.ignite.internal.mem.NumaAllocUtil;
+import org.apache.ignite.internal.util.tostring.GridToStringBuilder;
+
+/** */
+public class LocalNumaAllocationStrategy implements NumaAllocationStrategy, Serializable {
Review comment:
All parameters of IgniteConfiguration should be serializable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r758290871
##########
File path: modules/numa-allocator/src/main/cpp/src/numa/numa_alloc.cpp
##########
@@ -0,0 +1,196 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <numa.h>
+#include <numa/numa_alloc.h>
+
+namespace numa {
+ class BitSet::BitSetImpl {
+ public:
+ BitSetImpl() : size_(numa_max_node() + 1) {
+ mask_ = numa_bitmask_alloc(size_);
+ }
+
+ BitSetImpl(const BitSetImpl &other) : size_(other.size_) {
+ mask_ = numa_bitmask_alloc(size_);
+ copy_bitmask_to_bitmask(other.mask_, mask_);
+ }
+
+ void SetBit(size_t idx, bool val) {
+ if (idx < size_) {
+ if (val)
+ numa_bitmask_setbit(mask_, idx);
+ else
+ numa_bitmask_clearbit(mask_, idx);
+ }
+ }
+
+ void SetAll(bool val) {
+ if (val)
+ numa_bitmask_setall(mask_);
+ else
+ numa_bitmask_clearall(mask_);
+ }
+
+ bool GetBit(size_t idx) const {
+ if (idx < size_)
+ return numa_bitmask_isbitset(mask_, idx);
+ else
+ return false;
+ }
+
+ bool Equals(const BitSetImpl &other) const {
+ return numa_bitmask_equal(mask_, other.mask_);
+ }
+
+ size_t Size() const {
+ return size_;
+ }
+
+ bitmask *Raw() {
+ return mask_;
+ }
+
+ ~BitSetImpl() {
+ numa_bitmask_free(mask_);
+ }
+
+ private:
+ bitmask *mask_;
+ size_t size_;
+ };
+
+ BitSet::BitSet() {
+ p_impl_ = new BitSetImpl();
+ }
+
+ BitSet::BitSet(const BitSet &other) {
+ p_impl_ = new BitSetImpl(*other.p_impl_);
+ }
+
+ BitSet &BitSet::operator=(const BitSet &other) {
+ if (this != &other) {
+ BitSet tmp(other);
+ std::swap(this->p_impl_, tmp.p_impl_);
+ }
+ return *this;
+ }
+
+ bool BitSet::Get(size_t pos) const {
+ return p_impl_->GetBit(pos);
+ }
+
+ void BitSet::Set(size_t pos, bool value) {
+ p_impl_->SetBit(pos, value);
+ }
+
+ void BitSet::Set() {
+ p_impl_->SetAll(true);
+ }
+
+ void BitSet::Reset(size_t pos) {
+ p_impl_->SetBit(pos, false);
+ }
+
+ void BitSet::Reset() {
+ p_impl_->SetAll(false);
+ }
+
+ size_t BitSet::Size() const {
+ return p_impl_->Size();
+ }
+
+ bool BitSet::operator==(const BitSet &other) {
+ return this->p_impl_->Equals(*other.p_impl_);
+ }
+
+ bool BitSet::operator!=(const BitSet &other) {
+ return !(*this == other);
+ }
+
+ BitSet::~BitSet() {
+ delete p_impl_;
+ }
+
+ std::ostream &operator<<(std::ostream &os, const BitSet &set) {
+ os << '{';
+ for (size_t i = 0; i < set.Size(); ++i) {
+ os << set[i];
+ if (i < set.Size() - 1) {
+ os << ", ";
+ }
+ }
+ os << '}';
+ return os;
+ }
+
+ union region_size {
+ size_t size;
+ max_align_t a;
+ };
+
+ int NumaNodesCount() {
+ return numa_max_node() + 1;
+ }
+
+ template<typename Func, typename ...Args>
+ void *NumaAllocHelper(Func f, size_t size, Args ...args) {
+ auto ptr = static_cast<region_size *>(f(size + sizeof(region_size), args...));
Review comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] SammyVimes commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
SammyVimes commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r758123632
##########
File path: modules/numa-allocator/src/main/cpp/src/numa/numa_alloc.cpp
##########
@@ -0,0 +1,196 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <numa.h>
+#include <numa/numa_alloc.h>
+
+namespace numa {
+ class BitSet::BitSetImpl {
+ public:
+ BitSetImpl() : size_(numa_max_node() + 1) {
+ mask_ = numa_bitmask_alloc(size_);
+ }
+
+ BitSetImpl(const BitSetImpl &other) : size_(other.size_) {
+ mask_ = numa_bitmask_alloc(size_);
+ copy_bitmask_to_bitmask(other.mask_, mask_);
+ }
+
+ void SetBit(size_t idx, bool val) {
+ if (idx < size_) {
+ if (val)
+ numa_bitmask_setbit(mask_, idx);
+ else
+ numa_bitmask_clearbit(mask_, idx);
+ }
+ }
+
+ void SetAll(bool val) {
+ if (val)
+ numa_bitmask_setall(mask_);
+ else
+ numa_bitmask_clearall(mask_);
+ }
+
+ bool GetBit(size_t idx) const {
+ if (idx < size_)
+ return numa_bitmask_isbitset(mask_, idx);
+ else
+ return false;
+ }
+
+ bool Equals(const BitSetImpl &other) const {
+ return numa_bitmask_equal(mask_, other.mask_);
+ }
+
+ size_t Size() const {
+ return size_;
+ }
+
+ bitmask *Raw() {
+ return mask_;
+ }
+
+ ~BitSetImpl() {
+ numa_bitmask_free(mask_);
+ }
+
+ private:
+ bitmask *mask_;
+ size_t size_;
+ };
+
+ BitSet::BitSet() {
+ p_impl_ = new BitSetImpl();
+ }
+
+ BitSet::BitSet(const BitSet &other) {
+ p_impl_ = new BitSetImpl(*other.p_impl_);
+ }
+
+ BitSet &BitSet::operator=(const BitSet &other) {
+ if (this != &other) {
+ BitSet tmp(other);
+ std::swap(this->p_impl_, tmp.p_impl_);
+ }
+ return *this;
+ }
+
+ bool BitSet::Get(size_t pos) const {
+ return p_impl_->GetBit(pos);
+ }
+
+ void BitSet::Set(size_t pos, bool value) {
+ p_impl_->SetBit(pos, value);
+ }
+
+ void BitSet::Set() {
+ p_impl_->SetAll(true);
+ }
+
+ void BitSet::Reset(size_t pos) {
+ p_impl_->SetBit(pos, false);
+ }
+
+ void BitSet::Reset() {
+ p_impl_->SetAll(false);
+ }
+
+ size_t BitSet::Size() const {
+ return p_impl_->Size();
+ }
+
+ bool BitSet::operator==(const BitSet &other) {
+ return this->p_impl_->Equals(*other.p_impl_);
+ }
+
+ bool BitSet::operator!=(const BitSet &other) {
+ return !(*this == other);
+ }
+
+ BitSet::~BitSet() {
+ delete p_impl_;
+ }
+
+ std::ostream &operator<<(std::ostream &os, const BitSet &set) {
+ os << '{';
+ for (size_t i = 0; i < set.Size(); ++i) {
+ os << set[i];
+ if (i < set.Size() - 1) {
+ os << ", ";
+ }
+ }
+ os << '}';
+ return os;
+ }
+
+ union region_size {
+ size_t size;
+ max_align_t a;
+ };
+
+ int NumaNodesCount() {
+ return numa_max_node() + 1;
+ }
+
+ template<typename Func, typename ...Args>
+ void *NumaAllocHelper(Func f, size_t size, Args ...args) {
+ auto ptr = static_cast<region_size *>(f(size + sizeof(region_size), args...));
Review comment:
I think there should be a comment about the layout of a memory that is allocated.
##########
File path: modules/numa-allocator/pom.xml
##########
@@ -0,0 +1,178 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+<!--
+ POM file.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <parent>
+ <groupId>org.apache.ignite</groupId>
+ <artifactId>ignite-parent</artifactId>
+ <version>1</version>
+ <relativePath>../../parent</relativePath>
+ </parent>
+
+ <artifactId>ignite-numa-allocator</artifactId>
+ <version>${revision}</version>
+ <url>http://ignite.apache.org</url>
+
+ <dependencies>
+ <dependency>
+ <groupId>${project.groupId}</groupId>
+ <artifactId>ignite-core</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>${project.groupId}</groupId>
+ <artifactId>ignite-core</artifactId>
+ <version>${project.version}</version>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>${project.groupId}</groupId>
+ <artifactId>ignite-tools</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.springframework</groupId>
+ <artifactId>spring-beans</artifactId>
+ <version>${spring.version}</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.springframework</groupId>
+ <artifactId>spring-context</artifactId>
+ <version>${spring.version}</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>com.thoughtworks.xstream</groupId>
+ <artifactId>xstream</artifactId>
+ <version>1.4.8</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <version>${mockito.version}</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>log4j</groupId>
+ <artifactId>log4j</artifactId>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-antrun-plugin</artifactId>
Review comment:
Maybe https://github.com/cmake-maven-project/cmake-maven-project would be more suitable here. Not sure though
##########
File path: modules/numa-allocator/src/main/java/org/apache/ignite/internal/mem/NumaAllocUtil.java
##########
@@ -0,0 +1,153 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.mem;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Paths;
+
+/** */
+public class NumaAllocUtil {
+ /** */
+ private static final String extension = ".so";
+
+ /** */
+ private static final String libraryName = "libnuma_alloc";
+
+ /** */
+ private NumaAllocUtil() {
+ // No-op.
+ }
+
+ /** */
+ private static String getLibName() {
+ return Paths.get("/org/apache/ignite/internal/mem", getOSName(), getOSArch(), libraryName + extension)
+ .toString();
+ }
+
+ /** */
+ private static String getOSArch() {
+ return System.getProperty("os.arch", "");
+ }
+
+ /** */
+ private static String getOSName() {
+ if (System.getProperty("os.name", "").contains("Linux"))
+ return "linux";
+ else
+ throw new UnsupportedOperationException("Operating System is not supported");
+ }
+
+ static {
+ String libName = getLibName();
+ File nativeLib = null;
+
+ try (InputStream in = NumaAllocUtil.class.getResourceAsStream(libName)) {
+ if (in == null) {
+ throw new ExceptionInInitializerError("Failed to load native numa_alloc library, "
+ + libName + " not found");
+ }
+
+ nativeLib = File.createTempFile(libraryName, extension);
+
+ try (FileOutputStream out = new FileOutputStream(nativeLib)) {
Review comment:
Can be replaced with
```
nativeLib = Files.createTempFile(libraryName, extension);
Files.copy(in, nativeLib);
```
##########
File path: modules/numa-allocator/src/main/cpp/include/numa/numa_alloc.h
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef _NUMA_ALLOC_H
+#define _NUMA_ALLOC_H
+
+#include <ostream>
+
+namespace numa {
+ class BitSet {
+ public:
+ BitSet();
+
+ BitSet(const BitSet &other);
+
+ BitSet &operator=(const BitSet &other);
+
+ ~BitSet();
+
+ class Reference {
Review comment:
I don't quite get where this class is used. I see that it captures the position in a bitset and an object of the `Reference` is returned by the `[]` op. Is it used in the internals of the libnuma?
##########
File path: modules/numa-allocator/src/main/cpp/src/numa/numa_alloc.cpp
##########
@@ -0,0 +1,196 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <numa.h>
+#include <numa/numa_alloc.h>
+
+namespace numa {
+ class BitSet::BitSetImpl {
+ public:
+ BitSetImpl() : size_(numa_max_node() + 1) {
+ mask_ = numa_bitmask_alloc(size_);
+ }
+
+ BitSetImpl(const BitSetImpl &other) : size_(other.size_) {
+ mask_ = numa_bitmask_alloc(size_);
+ copy_bitmask_to_bitmask(other.mask_, mask_);
+ }
+
+ void SetBit(size_t idx, bool val) {
+ if (idx < size_) {
+ if (val)
+ numa_bitmask_setbit(mask_, idx);
+ else
+ numa_bitmask_clearbit(mask_, idx);
+ }
+ }
+
+ void SetAll(bool val) {
+ if (val)
+ numa_bitmask_setall(mask_);
+ else
+ numa_bitmask_clearall(mask_);
+ }
+
+ bool GetBit(size_t idx) const {
+ if (idx < size_)
+ return numa_bitmask_isbitset(mask_, idx);
+ else
+ return false;
+ }
+
+ bool Equals(const BitSetImpl &other) const {
+ return numa_bitmask_equal(mask_, other.mask_);
+ }
+
+ size_t Size() const {
+ return size_;
+ }
+
+ bitmask *Raw() {
+ return mask_;
+ }
+
+ ~BitSetImpl() {
+ numa_bitmask_free(mask_);
+ }
+
+ private:
+ bitmask *mask_;
+ size_t size_;
+ };
+
+ BitSet::BitSet() {
+ p_impl_ = new BitSetImpl();
+ }
+
+ BitSet::BitSet(const BitSet &other) {
+ p_impl_ = new BitSetImpl(*other.p_impl_);
+ }
+
+ BitSet &BitSet::operator=(const BitSet &other) {
+ if (this != &other) {
+ BitSet tmp(other);
+ std::swap(this->p_impl_, tmp.p_impl_);
+ }
+ return *this;
+ }
+
+ bool BitSet::Get(size_t pos) const {
+ return p_impl_->GetBit(pos);
+ }
+
+ void BitSet::Set(size_t pos, bool value) {
+ p_impl_->SetBit(pos, value);
+ }
+
+ void BitSet::Set() {
+ p_impl_->SetAll(true);
+ }
+
+ void BitSet::Reset(size_t pos) {
+ p_impl_->SetBit(pos, false);
+ }
+
+ void BitSet::Reset() {
+ p_impl_->SetAll(false);
+ }
+
+ size_t BitSet::Size() const {
+ return p_impl_->Size();
+ }
+
+ bool BitSet::operator==(const BitSet &other) {
+ return this->p_impl_->Equals(*other.p_impl_);
+ }
+
+ bool BitSet::operator!=(const BitSet &other) {
+ return !(*this == other);
+ }
+
+ BitSet::~BitSet() {
+ delete p_impl_;
+ }
+
+ std::ostream &operator<<(std::ostream &os, const BitSet &set) {
+ os << '{';
+ for (size_t i = 0; i < set.Size(); ++i) {
+ os << set[i];
+ if (i < set.Size() - 1) {
+ os << ", ";
+ }
+ }
+ os << '}';
+ return os;
+ }
+
+ union region_size {
+ size_t size;
+ max_align_t a;
+ };
+
+ int NumaNodesCount() {
+ return numa_max_node() + 1;
+ }
+
+ template<typename Func, typename ...Args>
+ void *NumaAllocHelper(Func f, size_t size, Args ...args) {
+ auto ptr = static_cast<region_size *>(f(size + sizeof(region_size), args...));
+ if (ptr) {
+ ptr->size = size;
+ ptr++;
+ }
+ return ptr;
+ }
+
+ void *Alloc(size_t size) {
+ return NumaAllocHelper(numa_alloc, size);
+ }
+
+ void *Alloc(size_t size, int node) {
+ return NumaAllocHelper(numa_alloc_onnode, size, node);
+ }
+
+ void *AllocLocal(size_t size) {
+ return NumaAllocHelper(numa_alloc_local, size);
+ }
+
+ void *AllocInterleaved(size_t size) {
+ return NumaAllocHelper(numa_alloc_interleaved, size);
+ }
+
+ void *AllocInterleaved(size_t size, const BitSet &node_set) {
+ return NumaAllocHelper(numa_alloc_interleaved_subset, size, node_set.p_impl_->Raw());
+ }
+
+ size_t Size(void *buf) {
+ if (buf) {
+ auto *ptr = static_cast<region_size *>(buf);
+ ptr--;
+ return ptr->size;
+ }
+ return 0;
+ }
+
+ void Free(void *buf) {
+ if (buf) {
+ auto *ptr = static_cast<region_size *>(buf);
+ ptr--;
Review comment:
I'd recommend writing a comment here and in `Size() ` about this cast and decrement, because ptr is not actually pointing to region_size before the decrement.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r758179512
##########
File path: modules/numa-allocator/src/main/cpp/include/numa/numa_alloc.h
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef _NUMA_ALLOC_H
+#define _NUMA_ALLOC_H
+
+#include <ostream>
+
+namespace numa {
+ class BitSet {
+ public:
+ BitSet();
+
+ BitSet(const BitSet &other);
+
+ BitSet &operator=(const BitSet &other);
+
+ ~BitSet();
+
+ class Reference {
Review comment:
Now it is not used. This is quite common technique (see i.e. bitset implementation in [libstdc++](https://github.com/gcc-mirror/gcc/blob/85e91ad55a69282c1b0e34569836a026a1a954d1/libstdc%2B%2B-v3/include/std/bitset#L802)) to implement non-const `operator[]`. With it you can do somethin like this
```c++
BitSet a, b;
a[0] = true;
for (size_t i = 0; i < a.Size() && i < b.Size(); ++i) {
a[i] = !b[i];
}
```
But this functionality is no used so far, it is for probably future uses
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] zstan commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
zstan commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r766614811
##########
File path: modules/numa-allocator/src/main/java/org/apache/ignite/internal/mem/NumaAllocUtil.java
##########
@@ -0,0 +1,151 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.mem;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Paths;
+
+/** */
+public class NumaAllocUtil {
+ /** */
+ private static final String extension = ".so";
+
+ /** */
+ private static final String libraryName = "libnuma_alloc";
+
+ /** */
+ private NumaAllocUtil() {
+ // No-op.
+ }
+
+ /** */
+ private static String getLibName() {
+ return Paths.get("/org/apache/ignite/internal/mem", getOSName(), getOSArch(), libraryName + extension)
+ .toString();
+ }
+
+ /** */
+ private static String getOSArch() {
+ return System.getProperty("os.arch", "");
+ }
+
+ /** */
+ private static String getOSName() {
Review comment:
who call this method ? Does Linux from uppercase is always correct?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #9569: IGNITE-15922 Implement NUMA-aware allocator
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #9569:
URL: https://github.com/apache/ignite/pull/9569#discussion_r766623534
##########
File path: modules/numa-allocator/src/main/java/org/apache/ignite/mem/InterleavedNumaAllocationStrategy.java
##########
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.mem;
+
+import java.io.Serializable;
+import java.util.Arrays;
+import org.apache.ignite.internal.mem.NumaAllocUtil;
+import org.apache.ignite.internal.util.tostring.GridToStringBuilder;
+import org.apache.ignite.internal.util.tostring.GridToStringInclude;
+import org.apache.ignite.internal.util.typedef.internal.A;
+
+/**
+ * Interleaved NUMA allocation strategy.
+ * <p>
+ * Use {@link InterleavedNumaAllocationStrategy#InterleavedNumaAllocationStrategy()} to allocate memory interleaved
+ * on all available NUMA nodes. Memory will be allocated using {@code void *numa_alloc_interleaved(size_t)}
+ * of {@code libnuma}.
+ * <p>
+ * Use {@link InterleavedNumaAllocationStrategy#InterleavedNumaAllocationStrategy(int[])} to allocate memory interleaved
+ * on specified nodes.
+ * {@code void *numa_alloc_interleaved_subset(size_t, struct bitmask*)} of {@code lubnuma}.
+ */
+public class InterleavedNumaAllocationStrategy implements NumaAllocationStrategy, Serializable {
+ /** */
+ private static final long serialVersionUID = 0L;
+
+ /** */
+ @GridToStringInclude
+ private final int[] nodes;
+
+ /** */
+ public InterleavedNumaAllocationStrategy() {
+ this(null);
+ }
+
+ /**
+ * @param nodes Array of NUMA nodes to allocate on.
+ */
+ public InterleavedNumaAllocationStrategy(int[] nodes) {
+ if (nodes != null && nodes.length > 0) {
+ this.nodes = Arrays.copyOf(nodes, nodes.length);
+
+ Arrays.sort(this.nodes);
+ A.ensure(this.nodes[0] >= 0, "NUMA node number must be positive, passed instead "
Review comment:
Because they are sorted
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org