matthiasblaesing commented on a change in pull request #3159: URL: https://github.com/apache/netbeans/pull/3159#discussion_r706827040
########## File path: profiler/lib.profiler/src/org/netbeans/lib/profiler/heap/Systems.java ########## @@ -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. + */ +package org.netbeans.lib.profiler.heap; + +final class Systems { + + //~ Static fields/initializers ----------------------------------------------------------------------------------------------- + static final boolean DEBUG = false; + + static boolean isLinux() { + String osName = System.getProperty("os.name"); // NOI18N + return osName.endsWith("Linux"); // NOI18N + } + + private Systems() { + } + + static void printStackTrace(Throwable t) { + t.printStackTrace(); + } + + static void printStackTrace(String msg, Throwable t) { + System.err.println(msg); + t.printStackTrace(System.err); + } + + static void debug(String msg) { Review comment: This looks more like a job for `java.util.logging.Logger#fine` - seeing `System.err` or System.out` used for debugging of something serious feels strange. ########## File path: profiler/lib.profiler/src/org/netbeans/lib/profiler/heap/Systems.java ########## @@ -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. + */ +package org.netbeans.lib.profiler.heap; + +final class Systems { + + //~ Static fields/initializers ----------------------------------------------------------------------------------------------- + static final boolean DEBUG = false; + + static boolean isLinux() { + String osName = System.getProperty("os.name"); // NOI18N + return osName.endsWith("Linux"); // NOI18N + } + + private Systems() { + } + + static void printStackTrace(Throwable t) { + t.printStackTrace(); + } + + static void printStackTrace(String msg, Throwable t) { + System.err.println(msg); Review comment: I suggest to used `java.util.logging.Logger#log(Level level, String message, Throwable t)`, without introducing a new helper. That way the exception is logged correctly, not spilled to STDERR (where it does not belong is not traceable). ########## File path: profiler/lib.profiler/src/org/netbeans/lib/profiler/heap/Progress.java ########## @@ -0,0 +1,114 @@ +/* + * Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package org.netbeans.lib.profiler.heap; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +enum Progress { + COMPUTE_INSTANCES, + COMPUTE_REFERENCES, + FILL_HEAP_TAG_BOUNDS, + COMPUTE_GC_ROOTS; + + Handle start() { + return new Handle(this); + } + + private static List<Listener> listeners = Collections.emptyList(); + synchronized static void register(Listener onChange) { + if (listeners.isEmpty()) { + listeners = Collections.singletonList(onChange); + } else { + List<Listener> copy = new ArrayList<>(listeners); + copy.add(onChange); + listeners = copy; + } + } + + synchronized static void notifyUpdates(Handle h, int type) { + for (Listener onChange : listeners) { + switch (type) { + case 1: onChange.started(h); break; + case 2: onChange.progress(h); break; + default: onChange.finished(h); + } + } + } + + static interface Listener { + void started(Handle h); + void progress(Handle h); + void finished(Handle h); + } + + static final class Handle implements AutoCloseable { + final Progress type; + private long value; + private long startOffset; + private long endOffset; + + private Handle(Progress type) { + this.type = type; + notifyUpdates(this, 1); + } + + void progress(long value, long endValue) { + progress(value, 0, value, endValue); + } + + void progress(long counter, long startOffset, long value, long endOffset) { + // keep this method short so that it can be inlined + if (counter % 100000 == 0) { + doProgress(value, startOffset, endOffset); + } + } + + @Override + public void close() { + notifyUpdates(this, 2); Review comment: This should from my reading of `notifyUpdates` either be `notifyUpdates(this, 0)` or `notifyUpdates(this, 3)` ########## File path: profiler/lib.profiler/src/org/netbeans/lib/profiler/heap/Systems.java ########## @@ -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. + */ +package org.netbeans.lib.profiler.heap; + +final class Systems { + + //~ Static fields/initializers ----------------------------------------------------------------------------------------------- + static final boolean DEBUG = false; + + static boolean isLinux() { + String osName = System.getProperty("os.name"); // NOI18N Review comment: I suggest to use `org.openide.util.BaseUtil.getOperatingSystem() == OS_LINUX` ########## File path: profiler/lib.profiler/src/org/netbeans/lib/profiler/heap/Progress.java ########## @@ -0,0 +1,114 @@ +/* + * Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package org.netbeans.lib.profiler.heap; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +enum Progress { + COMPUTE_INSTANCES, + COMPUTE_REFERENCES, + FILL_HEAP_TAG_BOUNDS, + COMPUTE_GC_ROOTS; + + Handle start() { + return new Handle(this); + } + + private static List<Listener> listeners = Collections.emptyList(); + synchronized static void register(Listener onChange) { + if (listeners.isEmpty()) { + listeners = Collections.singletonList(onChange); + } else { + List<Listener> copy = new ArrayList<>(listeners); + copy.add(onChange); + listeners = copy; + } + } + + synchronized static void notifyUpdates(Handle h, int type) { + for (Listener onChange : listeners) { + switch (type) { + case 1: onChange.started(h); break; + case 2: onChange.progress(h); break; + default: onChange.finished(h); + } + } + } + + static interface Listener { + void started(Handle h); + void progress(Handle h); + void finished(Handle h); + } + + static final class Handle implements AutoCloseable { + final Progress type; + private long value; + private long startOffset; + private long endOffset; + + private Handle(Progress type) { + this.type = type; + notifyUpdates(this, 1); + } + + void progress(long value, long endValue) { + progress(value, 0, value, endValue); + } + + void progress(long counter, long startOffset, long value, long endOffset) { + // keep this method short so that it can be inlined + if (counter % 100000 == 0) { + doProgress(value, startOffset, endOffset); + } + } + + @Override + public void close() { + notifyUpdates(this, 2); + } + + private void doProgress(long value, long startOffset, long endOffset) { + this.value = value; + this.endOffset = endOffset; + this.startOffset = startOffset; + notifyUpdates(this, 1); Review comment: I think reading the code in `notifyUpdates` this should be `notifyUpdates(this, 2)` ########## File path: profiler/lib.profiler/src/org/netbeans/lib/profiler/heap/Systems.java ########## @@ -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. + */ +package org.netbeans.lib.profiler.heap; + +final class Systems { + + //~ Static fields/initializers ----------------------------------------------------------------------------------------------- + static final boolean DEBUG = false; + + static boolean isLinux() { + String osName = System.getProperty("os.name"); // NOI18N + return osName.endsWith("Linux"); // NOI18N + } + + private Systems() { + } + + static void printStackTrace(Throwable t) { + t.printStackTrace(); Review comment: See comment in `printStackTrace(String msg, Throwable t)` - the logger can be invoked with an empty or even null message). ########## File path: profiler/lib.profiler/src/org/netbeans/lib/profiler/heap/Systems.java ########## @@ -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. + */ +package org.netbeans.lib.profiler.heap; + +final class Systems { + + //~ Static fields/initializers ----------------------------------------------------------------------------------------------- + static final boolean DEBUG = false; Review comment: This is not necessary - either the logger can be queries if it is loggable or the log call be be invoked directly. Last time I checked the first thing the log routines do is to check if a call can be discarded, because it will not yield visible output. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected] For further information about the NetBeans mailing lists, visit: https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
