On Thu, Jan 16, 2020 at 6:21 AM Waldemar Kozaczuk <[email protected]>
wrote:

> The java_api.* and jni_helpers.* are actually only used by
> java.cc and following httpserver java-related plugins:
> - httpserver-jolokia-plugin
> - httpserver-jvm-plugin
> - josvsym
>
> This patch moves the relevant code out from kernel
> and makes it part of java.so.
>

Looks good. I just have a couple of small question below.



> Signed-off-by: Waldemar Kozaczuk <[email protected]>
> ---
>  Makefile                                       |  2 --
>  modules/httpserver-api/Makefile                |  2 +-
>  modules/httpserver-api/module.py               |  1 +
>  modules/httpserver-jolokia-plugin/jolokia.cc   |  2 +-
>  modules/httpserver-jvm-plugin/jvm.cc           |  2 +-
>  modules/java-base/Makefile                     |  3 ++-
>  modules/java-base/common.gmk                   |  4 ++--
>  modules/java-base/java.cc                      |  4 ++--
>  {java => modules/java-base}/jvm/java_api.cc    |  0
>  {java => modules/java-base}/jvm/java_api.hh    |  0
>  {java => modules/java-base}/jvm/jni_helpers.cc |  0
>  {java => modules/java-base}/jvm/jni_helpers.hh |  0
>  modules/java-isolated/Makefile                 | 11 +++++++----
>  modules/java-non-isolated/Makefile             | 11 +++++++----
>  modules/java-tests/Makefile                    |  3 +++
>  modules/josvsym/josvsym.cc                     |  2 +-
>  16 files changed, 28 insertions(+), 19 deletions(-)
>  rename {java => modules/java-base}/jvm/java_api.cc (100%)
>  rename {java => modules/java-base}/jvm/java_api.hh (100%)
>  rename {java => modules/java-base}/jvm/jni_helpers.cc (100%)
>  rename {java => modules/java-base}/jvm/jni_helpers.hh (100%)
>
> diff --git a/Makefile b/Makefile
> index 62277d5f..51f76cd6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -817,8 +817,6 @@ drivers += drivers/clock-common.o
>  drivers += drivers/clockevent.o
>  drivers += core/elf.o
>  drivers += java/jvm/jvm_balloon.o
> -drivers += java/jvm/java_api.o
> -drivers += java/jvm/jni_helpers.o
>  drivers += drivers/random.o
>  drivers += drivers/zfs.o
>  drivers += drivers/null.o
> diff --git a/modules/httpserver-api/Makefile
> b/modules/httpserver-api/Makefile
> index 943fe705..525fafbf 100644
> --- a/modules/httpserver-api/Makefile
> +++ b/modules/httpserver-api/Makefile
> @@ -107,7 +107,7 @@ check-http:
>  check-ssl:
>         # Test SSL
>         cd $(src) && \
> -       make image=httpserver-api.fg_ssl,certs,openjdk8-zulu-full,jetty &&
> \
> +       make
> image=httpserver-api.daemon_ssl,certs,openjdk8-zulu-full,jetty && \
>

What is this change, from "fg_ssl" to "daemon_ssl", about?

        PYTHONPATH=$(src)/scripts
> modules/httpserver-api/tests/testhttpserver-api.py \
>                 --cert modules/certs/build/client.pem \
>                 --key modules/certs/build/client.key \
> diff --git a/modules/httpserver-api/module.py
> b/modules/httpserver-api/module.py
> index e1da7447..cdedf5a3 100644
> --- a/modules/httpserver-api/module.py
> +++ b/modules/httpserver-api/module.py
> @@ -22,6 +22,7 @@
> api.require_if_other_module_present('httpserver-jvm-plugin','java')
>  # httpserver will run regardless of an explicit command line
>  # passed with "run.py -e".
>  daemon = api.run_on_init(_exe + ' &!')
> +daemon_ssl = api.run_on_init(_exe + ' --ssl &!')
>
>  fg = api.run(_exe)
>
> diff --git a/modules/httpserver-jolokia-plugin/jolokia.cc
> b/modules/httpserver-jolokia-plugin/jolokia.cc
> index 30e3a7cb..75b451e9 100644
> --- a/modules/httpserver-jolokia-plugin/jolokia.cc
> +++ b/modules/httpserver-jolokia-plugin/jolokia.cc
> @@ -15,7 +15,7 @@
>  #include <mutex>
>  #include <memory>
>  #include <regex>
> -#include <java/jvm/jni_helpers.hh>
> +#include "../java-base/jvm/jni_helpers.hh"
>  #include "exception.hh"
>
>  using namespace httpserver::json;
> diff --git a/modules/httpserver-jvm-plugin/jvm.cc
> b/modules/httpserver-jvm-plugin/jvm.cc
> index 68c3760e..b27481ce 100644
> --- a/modules/httpserver-jvm-plugin/jvm.cc
> +++ b/modules/httpserver-jvm-plugin/jvm.cc
> @@ -7,7 +7,7 @@
>
>  #include "jvm.hh"
>  #include "autogen/jvm.json.hh"
> -#include "jvm/java_api.hh"
> +#include "../java-base/jvm/java_api.hh"
>  #include <string>
>
>  namespace httpserver {
> diff --git a/modules/java-base/Makefile b/modules/java-base/Makefile
> index 13c1858c..d61999cf 100644
> --- a/modules/java-base/Makefile
> +++ b/modules/java-base/Makefile
> @@ -3,7 +3,7 @@ include common.gmk
>  ifeq ($(arch),aarch64)
>  java-targets :=
>  else
> -java-targets := obj/jni/monitor.so
> +java-targets := obj/jni/monitor.so obj/jvm/jni_helpers.o
> obj/jvm/java_api.o
>  endif
>
>  module: all
> @@ -13,6 +13,7 @@ all: $(init) $(java-targets)
>  init:
>         @echo "  MKDIRS"
>         $(call very-quiet, mkdir -p obj/jni)
> +       $(call very-quiet, mkdir -p obj/jvm)
>  .PHONY: init
>
>  clean:
> diff --git a/modules/java-base/common.gmk b/modules/java-base/common.gmk
> index e54a301a..0e2e4690 100644
> --- a/modules/java-base/common.gmk
> +++ b/modules/java-base/common.gmk
> @@ -1,7 +1,5 @@
> -
>  INCLUDES = -I$(src)/arch/$(arch) -I$(src) -I$(src)/include
> -I$(src)/arch/common
>  INCLUDES += -I$(src)/include/glibc-compat
> -INCLUDES += -isystem $(src)/external/$(arch)/acpica/source/include
>

What is this change about? I guess you just noticed it's not needed?

 INCLUDES += $(shell $(CXX) -E -xc++ - -v </dev/null 2>&1 | awk '/^End/
> {exit} /^ .*c\+\+/ {print "-isystem" $$0}')
>  INCLUDES += -isystem $(src)/include/api
>  INCLUDES += -isystem $(src)/include/api/$(arch)
> @@ -13,7 +11,9 @@ autodepend = -MD -MT $@ -MP
>  COMMON_FLAGS  = -g -Wall -fPIC $(INCLUDES) -O2 $(autodepend)
> -DCONF_debug_memory=0 -D_KERNEL
>  CXXFLAGS  = -std=c++11 $(COMMON_FLAGS)
>  CFLAGS = -std=gnu99 $(COMMON_FLAGS)
> +
>  src = $(shell readlink -f ../..)
> +java-base-path := $(src)/modules/java-base
>
>  RM := /bin/rm
>
> diff --git a/modules/java-base/java.cc b/modules/java-base/java.cc
> index 74e0d860..796f6e44 100644
> --- a/modules/java-base/java.cc
> +++ b/modules/java-base/java.cc
> @@ -14,9 +14,9 @@
>  #include <osv/debug.hh>
>  #include <java/jvm/jvm_balloon.hh>
>  #include <osv/mempool.hh>
> -#include <java/jvm/java_api.hh>
> +#include "jvm/java_api.hh"
>  #include "osv/version.hh"
> -#include <java/jvm/jni_helpers.hh>
> +#include "jvm/jni_helpers.hh"
>  #include <osv/defer.hh>
>  #include <osv/app.hh>
>
> diff --git a/java/jvm/java_api.cc b/modules/java-base/jvm/java_api.cc
> similarity index 100%
> rename from java/jvm/java_api.cc
> rename to modules/java-base/jvm/java_api.cc
> diff --git a/java/jvm/java_api.hh b/modules/java-base/jvm/java_api.hh
> similarity index 100%
> rename from java/jvm/java_api.hh
> rename to modules/java-base/jvm/java_api.hh
> diff --git a/java/jvm/jni_helpers.cc b/modules/java-base/jvm/jni_helpers.cc
> similarity index 100%
> rename from java/jvm/jni_helpers.cc
> rename to modules/java-base/jvm/jni_helpers.cc
> diff --git a/java/jvm/jni_helpers.hh b/modules/java-base/jvm/jni_helpers.hh
> similarity index 100%
> rename from java/jvm/jni_helpers.hh
> rename to modules/java-base/jvm/jni_helpers.hh
> diff --git a/modules/java-isolated/Makefile
> b/modules/java-isolated/Makefile
> index 549aed5f..e0e15a01 100644
> --- a/modules/java-isolated/Makefile
> +++ b/modules/java-isolated/Makefile
> @@ -8,17 +8,20 @@ else
>  java-targets := obj/java.so
>  endif
>
> -obj/java.o: $(SRC)/modules/java-base/java.cc | init
> -       $(call quiet, $(CXX) $(CXXFLAGS) -o $@ -c
> $(SRC)/modules/java-base/java.cc -MMD, CXX $@)
> +obj/java.o: $(java-base-path)/java.cc | init
> +       $(call quiet, $(CXX) $(CXXFLAGS) -o $@ -c
> $(java-base-path)/java.cc -MMD, CXX $@)
> +
> +obj/java.so: obj/java.o $(java-base-path)/obj/jvm/java_api.o
> $(java-base-path)/obj/jvm/jni_helpers.o
> +       $(call quiet, $(CXX) $(CXXFLAGS) -shared -o $@ $^, LINK $@)
>
>  init:
>         @echo "  MKDIRS"
>         $(call very-quiet, mkdir -p obj)
>
>  module: $(java-targets)
> -       cd $(SRC)/modules/java-base && mvn -q --projects
> :runjava-common,:runjava-isolated  package -DskipTests=true
> +       cd $(java-base-path) && mvn -q --projects
> :runjava-common,:runjava-isolated  package -DskipTests=true
>
>  clean:
> -       cd $(SRC)/modules/java-base && mvn -q clean
> +       cd $(java-base-path) && mvn -q clean
>         -rm -f dependency-reduced-pom.xml
>         $(call very-quiet, $(RM) -rf obj)
> diff --git a/modules/java-non-isolated/Makefile
> b/modules/java-non-isolated/Makefile
> index bf87f6d7..19e4a48f 100644
> --- a/modules/java-non-isolated/Makefile
> +++ b/modules/java-non-isolated/Makefile
> @@ -8,17 +8,20 @@ else
>  java-targets := obj/java_non_isolated.so
>  endif
>
> -obj/java_non_isolated.o: $(SRC)/modules/java-base/java.cc | init
> -       $(call quiet, $(CXX) $(CXXFLAGS) -DRUN_JAVA_NON_ISOLATED -o $@ -c
> $(SRC)/modules/java-base/java.cc -MMD, CXX $@)
> +obj/java_non_isolated.o: $(java-base-path)/java.cc | init
> +       $(call quiet, $(CXX) $(CXXFLAGS) -DRUN_JAVA_NON_ISOLATED -o $@ -c
> $(java-base-path)/java.cc -MMD, CXX $@)
> +
> +obj/java_non_isolated.so: obj/java_non_isolated.o
> $(java-base-path)/obj/jvm/java_api.o $(java-base-path)/obj/jvm/jni_helpers.o
> +       $(call quiet, $(CXX) $(CXXFLAGS) -shared -o $@ $^, LINK $@)
>
>  init:
>         @echo "  MKDIRS"
>         $(call very-quiet, mkdir -p obj)
>
>  module: $(java-targets)
> -       cd $(SRC)/modules/java-base && mvn -q --projects
> :runjava-common,:runjava-non-isolated package -DskipTests=true
> +       cd $(java-base-path) && mvn -q --projects
> :runjava-common,:runjava-non-isolated package -DskipTests=true
>
>  clean:
> -       cd $(SRC)/modules/java-base && mvn -q clean
> +       cd $(java-base-path) && mvn -q clean
>         -rm -f dependency-reduced-pom.xml
>         $(call very-quiet, $(RM) -rf obj)
> diff --git a/modules/java-tests/Makefile b/modules/java-tests/Makefile
> index 874f3bad..dc3dbb1b 100644
> --- a/modules/java-tests/Makefile
> +++ b/modules/java-tests/Makefile
> @@ -10,6 +10,9 @@ endif
>  obj/java_isolated.o: $(SRC)/modules/java-base/java.cc | init
>         $(call quiet, $(CXX) $(CXXFLAGS) -o $@ -c
> $(SRC)/modules/java-base/java.cc -MMD, CXX $@)
>
> +obj/java_isolated.so: obj/java_isolated.o
> $(java-base-path)/obj/jvm/java_api.o $(java-base-path)/obj/jvm/jni_helpers.o
> +       $(call quiet, $(CXX) $(CXXFLAGS) -shared -o $@ $^, LINK $@)
> +
>  init:
>         @echo "  MKDIRS"
>         $(call very-quiet, mkdir -p obj)
> diff --git a/modules/josvsym/josvsym.cc b/modules/josvsym/josvsym.cc
> index 50638298..c74a713e 100644
> --- a/modules/josvsym/josvsym.cc
> +++ b/modules/josvsym/josvsym.cc
> @@ -5,7 +5,7 @@
>  #include <thread>
>  #include <initializer_list>
>  #include <osv/tracecontrol.hh>
> -#include <java/jvm/jni_helpers.hh>
> +#include "../java-base/jvm/jni_helpers.hh"
>
>  static jvmtiEnv * jvmti;
>  static trace::generator_id id;
> --
> 2.20.1
>
> --
> You received this message because you are subscribed to the Google Groups
> "OSv Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/osv-dev/20200116042111.4833-1-jwkozaczuk%40gmail.com
> .
>

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/osv-dev/CANEVyjscm8EUvkphAgM%2B3Q_b%3DY5XQAZm1VA6L9CwyZK8Thq-5w%40mail.gmail.com.

Reply via email to