Skip to content

Literal storage #148

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

Merged
merged 3 commits into from
Jun 10, 2015
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Implement recordset iterators
JerryScript-DCO-1.0-Signed-off-by: Evgeny Gavrin e.gavrin@samsung.com
JerryScript-DCO-1.0-Signed-off-by: Andrey Shitov a.shitov@samsung.com
  • Loading branch information
sand1k committed Jun 10, 2015
commit 528a333e172ca27f613d049f7bbc3957ba8df671
24 changes: 16 additions & 8 deletions jerry-core/rcs/rcs-chunked-list.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@
class rcs_chunked_list_t
{
public:
/**
* Constructor of chunked list
*/
rcs_chunked_list_t ()
{
head_p = tail_p = NULL;
} /* rcs_chunked_list_t */

/**
* List node
*/
Expand All @@ -52,27 +60,27 @@ class rcs_chunked_list_t
node_t *get_first (void) const;
node_t *get_last (void) const;

node_t *get_prev (node_t *node_p) const;
node_t *get_prev (node_t *) const;
node_t *get_next (node_t *node_p) const;

node_t *append_new (void);
node_t *insert_new (node_t *after_p);
node_t *insert_new (node_t *);

void remove (node_t *node_p);
void remove (node_t *);

node_t *get_node_from_pointer (void *ptr) const;
uint8_t* get_data_space (node_t *node_p) const;
node_t *get_node_from_pointer (void *) const;
uint8_t* get_data_space (node_t *) const;

static size_t get_data_space_size (void);

private:
void set_prev (node_t *node_p, node_t *prev_node_p);
void set_next (node_t *node_p, node_t *next_node_p);
void set_prev (node_t *, node_t *);
void set_next (node_t *, node_t *);

static size_t get_node_size (void);

void assert_list_is_correct (void) const;
void assert_node_is_correct (const node_t *node_p) const;
void assert_node_is_correct (const node_t *) const;

node_t* head_p; /**< head node of list */
node_t* tail_p; /**< tail node of list */
Expand Down
120 changes: 118 additions & 2 deletions jerry-core/rcs/rcs-recordset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

#include "rcs-chunked-list.h"
#include "rcs-recordset.h"
#include "jrt-bit-fields.h"

/** \addtogroup recordset Recordset
* @{
Expand Down Expand Up @@ -91,7 +90,6 @@ rcs_recordset_t::record_t::cpointer_t::decompress (rcs_cpointer_t compressed_poi
* compressed pointer */
{
uint8_t* base_pointer;

if (compressed_pointer.value.base_cp == MEM_CP_NULL)
{
base_pointer = NULL;
Expand All @@ -106,6 +104,19 @@ rcs_recordset_t::record_t::cpointer_t::decompress (rcs_cpointer_t compressed_poi
return (rcs_recordset_t::record_t*) (base_pointer + diff);
} /* rcs_recordset_t::record_t::cpointer_t::decompress */

/**
* Create NULL compressed pointer
*
* @return NULL compressed pointer
*/
rcs_cpointer_t
rcs_recordset_t::record_t::cpointer_t::null_cp ()
{
rcs_cpointer_t cp;
cp.packed_value = MEM_CP_NULL;
return cp;
} /* rcs_recordset_t::record_t::cpointer_t::null_cp */

/**
* Assert that 'this' value points to correct record
*/
Expand Down Expand Up @@ -612,6 +623,7 @@ rcs_recordset_t::assert_state_is_correct (void)
rec_p != NULL;
last_record_p = rec_p, rec_p = next_rec_p)
{
JERRY_ASSERT (get_record_size (rec_p) > 0);
record_size_sum += get_record_size (rec_p);

rcs_chunked_list_t::node_t *node_p = _chunk_list.get_node_from_pointer (rec_p);
Expand Down Expand Up @@ -649,6 +661,110 @@ rcs_recordset_t::assert_state_is_correct (void)
#endif /* !JERRY_NDEBUG */
} /* rcs_recordset_t::assert_state_is_correct */

/**
* Perform general access to the record
*
* Warning: This function is implemented in assumption that `size` is not more than `2 * node_data_space_size`.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we move the comments to definition of access_t, or maybe duplicate them there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed

void
rcs_record_iterator_t::access (access_t access_type, /**< type of access: read, write or skip */
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm sure that the record iterator implementation should be placed in a separate commit.

void *data, /**< in/out data to read or write */
size_t size) /**< size of the data in bytes */
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems that the function is implemented in assumption that size is not more than 2 * node_data_space_size.
If it is correct, we need a related warning in the interface description, in the read, write, and skip and assertion check here.

Copy link
Contributor

Choose a reason for hiding this comment

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

@sand1k is it correct assertion?

  const size_t node_data_space_size = _recordset_p->_chunk_list.get_data_space_size ();
  JERRY_ASSERT(2 * node_data_space_size >= size);
  const size_t record_size = _recordset_p->get_record_size (_record_start_p);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed

{
const size_t node_data_space_size = _recordset_p->_chunk_list.get_data_space_size ();
JERRY_ASSERT (2 * node_data_space_size >= size);
const size_t record_size = _recordset_p->get_record_size (_record_start_p);

JERRY_ASSERT (!finished ());

rcs_chunked_list_t::node_t *current_node_p = _recordset_p->_chunk_list.get_node_from_pointer (_current_pos_p);
uint8_t *current_node_data_space_p = _recordset_p->_chunk_list.get_data_space (current_node_p);
size_t left_in_node = node_data_space_size - (size_t)(_current_pos_p - current_node_data_space_p);

JERRY_ASSERT (_current_offset + size <= record_size);

/*
* Read the data and increase the current position pointer.
*/
if (left_in_node >= size)
{
/* all data is placed inside single node */
if (access_type == ACCESS_READ)
{
memcpy (data, _current_pos_p, size);
}
else if (access_type == ACCESS_WRITE)
{
memcpy (_current_pos_p, data, size);
}
else
{
Copy link
Contributor

Choose a reason for hiding this comment

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

JERRY_ASSERT (access_type == ACCESS_SKIP);

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed

JERRY_ASSERT (access_type == ACCESS_SKIP);

if (left_in_node > size)
{
_current_pos_p += size;
}
else if (_current_offset + size < record_size)
{
current_node_p = _recordset_p->_chunk_list.get_next (current_node_p);
JERRY_ASSERT (current_node_p);
_current_pos_p = _recordset_p->_chunk_list.get_data_space (current_node_p);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

else? Please, add related assertion or comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ruben-ayrapetyan which one? JERRY_UNREACHABLE?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think, JERRY_ASSERT is better, as it is removed in release mode, and JERRY_UNREACHABLE - is not.

Maybe, something like:

      if (left_in_node > size)
      {
        _current_pos_p += size;
      }
      else
      {
        JERRY_ASSERT (current_offset + size < record_size);

        current_node_p = _recordset_p->_chunk_list.get_next (current_node_p);
        JERRY_ASSERT (current_node_p);
        _current_pos_p = _recordset_p->_chunk_list.get_data_space (current_node_p);
      }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It shouldn't be in assert. Situation where current_offset + size == record_size is valid.
In this case we are reading the last portion of the last chunk in the list, so we shouldn't update _current_node_p and _current_pos_p.
_current_pos_p will be set to NULL at the end of the function in this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

In the case, seems that we need assertion current_offset + size == record_size in else case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ruben-ayrapetyan, I agree.

Copy link
Contributor

Choose a reason for hiding this comment

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

else
{
JERRY_ASSERT (_current_offset + size == record_size);
}
}
}
else
{
/* data is distributed between two nodes */
size_t first_chunk_size = node_data_space_size - (size_t) (_current_pos_p - current_node_data_space_p);

if (access_type == ACCESS_READ)
{
memcpy (data, _current_pos_p, first_chunk_size);
}
else if (access_type == ACCESS_WRITE)
{
memcpy (_current_pos_p, data, first_chunk_size);
}

rcs_chunked_list_t::node_t *next_node_p = _recordset_p->_chunk_list.get_next (current_node_p);
JERRY_ASSERT (next_node_p != NULL);
uint8_t *next_node_data_space_p = _recordset_p->_chunk_list.get_data_space (next_node_p);

if (access_type == ACCESS_READ)
{
memcpy ((uint8_t *)data + first_chunk_size, next_node_data_space_p, size - first_chunk_size);
}
else if (access_type == ACCESS_WRITE)
{
memcpy (next_node_data_space_p, (uint8_t *)data + first_chunk_size, size - first_chunk_size);
}
else
{
JERRY_ASSERT (access_type == ACCESS_SKIP);

_current_pos_p = next_node_data_space_p + size - first_chunk_size;
}
}

/* check if we reached the end */
if (access_type == ACCESS_SKIP)
{
_current_offset += size;
JERRY_ASSERT (_current_offset <= record_size);

if (_current_offset == record_size)
{
_current_pos_p = NULL;
_current_offset = 0;
}
}
} /* rcs_record_iterator_t::access */

/**
* Get size of the free record
*/
Expand Down
Loading