Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8271569: Clean up the use of CDS constants and field offsets #5450

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,71 @@
/*
* Copyright (c) 2014, 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.
*
* 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.
*
*/

#include "precompiled.hpp"

#include "cds.h"
#include "cds/cdsConstants.hpp"
#include "cds/dynamicArchive.hpp"
#include "cds/filemap.hpp"
#include "utilities/globalDefinitions.hpp"

CDSConst CDSConstants::offsets[] = {
{ "CDSFileMapHeaderBase::_magic", offset_of(CDSFileMapHeaderBase, _magic) },
{ "CDSFileMapHeaderBase::_crc", offset_of(CDSFileMapHeaderBase, _crc) },
{ "CDSFileMapHeaderBase::_version", offset_of(CDSFileMapHeaderBase, _version) },
{ "CDSFileMapHeaderBase::_space[0]", offset_of(CDSFileMapHeaderBase, _space) },
{ "FileMapHeader::_jvm_ident", offset_of(FileMapHeader, _jvm_ident) },
{ "FileMapHeader::_base_archive_name_size", offset_of(FileMapHeader, _base_archive_name_size) },
{ "CDSFileMapRegion::_crc", offset_of(CDSFileMapRegion, _crc) },
{ "CDSFileMapRegion::_used", offset_of(CDSFileMapRegion, _used) },
{ "DynamicArchiveHeader::_base_region_crc", offset_of(DynamicArchiveHeader, _base_region_crc) }
};

CDSConst CDSConstants::constants[] = {
{ "static_magic", (size_t)CDS_ARCHIVE_MAGIC },
{ "dynamic_magic", (size_t)CDS_DYNAMIC_ARCHIVE_MAGIC },
{ "int_size", sizeof(int) },
{ "CDSFileMapRegion_size", sizeof(CDSFileMapRegion) },
{ "static_file_header_size", sizeof(FileMapHeader) },
{ "dynamic_archive_header_size", sizeof(DynamicArchiveHeader) },
{ "size_t_size", sizeof(size_t) }
};

size_t CDSConstants::get_cds_offset(const char* name) {
for (int i = 0; i < (int)ARRAY_SIZE(offsets); i++) {
if (strcmp(name, offsets[i]._name) == 0) {
return offsets[i]._value;
}
}
return -1;
}

size_t CDSConstants::get_cds_constant(const char* name) {
for (int i = 0; i < (int)ARRAY_SIZE(constants); i++) {
if (strcmp(name, constants[i]._name) == 0) {
return constants[i]._value;
}
}
return -1;
}
@@ -22,26 +22,21 @@
*
*/

#ifndef SHARE_CDS_CDSOFFSETS_HPP
#define SHARE_CDS_CDSOFFSETS_HPP
#ifndef SHARE_CDS_CDSCONSTANTS_HPP
#define SHARE_CDS_CDSCONSTANTS_HPP

#include "memory/allocation.hpp"
#include "memory/allStatic.hpp"
typedef struct {
const char* _name;
size_t _value;
} CDSConst;

class CDSOffsets: public CHeapObj<mtInternal> {
class CDSConstants : AllStatic {
private:
char* _name;
int _offset;
CDSOffsets* _next;
static CDSOffsets* _all; // sole list for cds
static CDSConst offsets[];
static CDSConst constants[];
yminqi marked this conversation as resolved.
Show resolved Hide resolved
public:
CDSOffsets(const char* name, int offset, CDSOffsets* next);

char* get_name() const { return _name; }
int get_offset() const { return _offset; }
CDSOffsets* next() const { return _next; }
void add_end(CDSOffsets* n);

static int find_offset(const char* name);
static size_t get_cds_constant(const char* name);
static size_t get_cds_offset(const char* name);
};

#endif // SHARE_CDS_CDSOFFSETS_HPP
#endif // SHARE_CDS_CDSCONSTANTS_HPP

This file was deleted.

@@ -38,7 +38,7 @@
#if INCLUDE_CDS

class DynamicArchiveHeader : public FileMapHeader {
friend class CDSOffsets;
friend class CDSConstants;
private:
int _base_header_crc;
int _base_region_crc[MetaspaceShared::n_regions];
yminqi marked this conversation as resolved.
Show resolved Hide resolved
@@ -180,9 +180,10 @@ class FileMapRegion: private CDSFileMapRegion {
};

class FileMapHeader: private CDSFileMapHeaderBase {
friend class CDSOffsets;
friend class CDSConstants;
Copy link
Member

@iklam iklam Sep 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to sort the classes alphabetically. CDSConstants should come before VMStructs.

friend class VMStructs;

private:
size_t _header_size;

// The following fields record the states of the VM during dump time.
@@ -209,7 +210,6 @@ class FileMapHeader: private CDSFileMapHeaderBase {
// will function correctly with this JVM and the bootclasspath it's
// invoked with.
char _jvm_ident[JVM_IDENT_MAX]; // identifier string of the jvm that created this dump

// size of the base archive name including NULL terminator
size_t _base_archive_name_size;

@@ -24,7 +24,7 @@

#include "precompiled.hpp"
#include <new>
#include "cds/cdsoffsets.hpp"
#include "cds/cdsConstants.hpp"
#include "cds/filemap.hpp"
#include "cds/heapShared.inline.hpp"
#include "cds/metaspaceShared.hpp"
@@ -2016,11 +2016,18 @@ WB_END

#if INCLUDE_CDS

WB_ENTRY(jint, WB_GetOffsetForName(JNIEnv* env, jobject o, jstring name))
WB_ENTRY(jint, WB_GetCDSOffsetForName(JNIEnv* env, jobject o, jstring name))
ResourceMark rm;
char* c_name = java_lang_String::as_utf8_string(JNIHandles::resolve_non_null(name));
int result = CDSOffsets::find_offset(c_name);
return (jint)result;
jint result = (jint)CDSConstants::get_cds_offset(c_name);
return result;
WB_END

WB_ENTRY(jint, WB_GetCDSConstantForName(JNIEnv* env, jobject o, jstring name))
ResourceMark rm;
char* c_name = java_lang_String::as_utf8_string(JNIHandles::resolve_non_null(name));
jint result = (jint)CDSConstants::get_cds_constant(c_name);
return result;
WB_END

#endif // INCLUDE_CDS
@@ -2457,7 +2464,8 @@ static JNINativeMethod methods[] = {
{CC"readFromNoaccessArea",CC"()V", (void*)&WB_ReadFromNoaccessArea},
{CC"stressVirtualSpaceResize",CC"(JJJ)I", (void*)&WB_StressVirtualSpaceResize},
#if INCLUDE_CDS
{CC"getOffsetForName0", CC"(Ljava/lang/String;)I", (void*)&WB_GetOffsetForName},
{CC"getCDSOffsetForName0", CC"(Ljava/lang/String;)I", (void*)&WB_GetCDSOffsetForName},
{CC"getCDSConstantForName0", CC"(Ljava/lang/String;)I", (void*)&WB_GetCDSConstantForName},
#endif
#if INCLUDE_G1GC
{CC"g1InConcurrentMark", CC"()Z", (void*)&WB_G1InConcurrentMark},