test/table: skip when not enough memory is available

Message ID 1562318340-30464-1-git-send-email-david.marchand@redhat.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers
Series test/table: skip when not enough memory is available |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

David Marchand July 5, 2019, 9:19 a.m. UTC
  Following empirical tries, this test wants at least 3300M of memory to
run.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/test_table.c | 7 +++++++
 1 file changed, 7 insertions(+)
  

Comments

Cristian Dumitrescu July 5, 2019, 5:54 p.m. UTC | #1
Hi David,

> -----Original Message-----
> From: David Marchand [mailto:david.marchand@redhat.com]
> Sent: Friday, July 5, 2019 10:19 AM
> To: dev@dpdk.org
> Cc: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Subject: [PATCH] test/table: skip when not enough memory is available
> 
> Following empirical tries, this test wants at least 3300M of memory to
> run.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  app/test/test_table.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/app/test/test_table.c b/app/test/test_table.c
> index a4b0ed6..dca1b33 100644
> --- a/app/test/test_table.c
> +++ b/app/test/test_table.c
> @@ -134,6 +134,13 @@ uint32_t pipeline_test_hash_cuckoo(const void
> *key,
>  {
>  	int status, ret;
>  	unsigned i;
> +	void *p;
> +
> +	/* empirical value, please refine when updating this test */
> +	p = rte_malloc("enough_memory_for_test", (size_t)3300 << 20, 0);
> +	if (!p)
> +		return TEST_SKIPPED;
> +	rte_free(p);
> 
>  	ret = TEST_SUCCESS;
> 
> --
> 1.8.3.1

Yes, some tests need large amounts of memory.

I understand your proposal, but it feels a bit like a hack for this test. Could we create a more generic fix?

Expanding on your idea, could we wrap this code into a macro such as TEST_CHECK_FOR_LARGE_MEMORY(memsize_in_gb) that could live in test.h and call it for this test (and potentially others that have the same requirements)?

Or maybe people have a better idea?

Thanks,
Cristian
  
Thomas Monjalon July 5, 2019, 8:47 p.m. UTC | #2
Hi,

05/07/2019 19:54, Dumitrescu, Cristian:
> From: David Marchand [mailto:david.marchand@redhat.com]
> > Following empirical tries, this test wants at least 3300M of memory to
> > run.
> > 
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> 
> Yes, some tests need large amounts of memory.

Basic tests should not.

> I understand your proposal, but it feels
> a bit like a hack for this test.

Of course it is a hack, probably on purpose to show the issue :)

> Could we create a more generic fix?
> 
> Expanding on your idea, could we wrap this code
> into a macro such as TEST_CHECK_FOR_LARGE_MEMORY(memsize_in_gb)
> that could live in test.h and call it for this test
> (and potentially others that have the same requirements)?

I don't see what it would improve.

> Or maybe people have a better idea?

Yes, a better idea is to reduce the requirements.
Requiring more than 3GB for a unit test is not reasonnable.
If you want to stress the algorithm, then it is a performance test,
and it won't be launched on basic CI.

So there are 2 proposals here:
	- use small memory amount
	- classify as perf test
Please choose one of these two, but neither the hack above,
nor a macro to hide the hack.
Thanks
  

Patch

diff --git a/app/test/test_table.c b/app/test/test_table.c
index a4b0ed6..dca1b33 100644
--- a/app/test/test_table.c
+++ b/app/test/test_table.c
@@ -134,6 +134,13 @@  uint32_t pipeline_test_hash_cuckoo(const void *key,
 {
 	int status, ret;
 	unsigned i;
+	void *p;
+
+	/* empirical value, please refine when updating this test */
+	p = rte_malloc("enough_memory_for_test", (size_t)3300 << 20, 0);
+	if (!p)
+		return TEST_SKIPPED;
+	rte_free(p);
 
 	ret = TEST_SUCCESS;