jackrabbit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Felix Meschberger <fmesc...@gmail.com>
Subject Re: [jira] Updated: (JCR-1232) Merge UUID to NodeId
Date Mon, 26 Nov 2007 08:30:24 GMT
Hi all,

(Taking this discussion to the list, which is probably more appropriate
for discussion and easier to follow than just expanding the issue)

I am definitely +1 for cleaning this stuff up.

Of course Jukka's patch probably misses the goal of not creating two
objects per created NodeId. It is just so, that the UUID object created
is not kept any longer, but dropped immediately. It is even worse:
calling NodeId.toString() creates a new throway-UUID and String object
on each call (Depending on how short-lived NodeId objects actually are,
caching the string value might be worth it...)

I would NOT extend the UUID class (as proposed by Thomas). As we all
know Java 5 finally introduces a UUID class in its core and Java 1.4
will sooner or later leave the planet, so would probably also the
jackrabbit implementation. If NodeId would extend UUID, this could not
be done. In addtion a NodeId is not a UUID it just uses a UUID. So -1 to
this proposal.

After all, Jukka's patch reaches one very important goal: The use of a
UUID is kept strictily internal to the NodeId class. This is a good
thing not only regarding JSR-283 but also in terms of good practices,
whereby hiding the concrete implementation of some functionality is
almost always a good thing. Having NodeId extend UUID would not reach
this goal and fail blatantly in this respect.

Regards
Felix

Am Donnerstag, den 22.11.2007, 18:26 -0800 schrieb Jukka Zitting (JIRA):
> [ https://issues.apache.org/jira/browse/JCR-1232?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
> 
> Jukka Zitting updated JCR-1232:
> -------------------------------
> 
>     Attachment: nodeid.patch
> 
> Attached a patch that merges NodeId and UUID. The patch is pretty straightforward and
should cause no functional changes, but given the size of the patch and the central role of
NodeId I wanted to put the patch up for review before committing it.
> 
> The main potential concerns that I could think of are the changed serialization format
of NodeId and the removed SessionImpl.getNodeByUUID(UUID) method.
> 
> All of our persistence managers use a custom serialization mechanisms, so the change
in the Java serialization format of NodeId should not cause problems.
> 
> I removed the SessionImpl.getNodeByUUID(UUID) method since we already have the standard
Session.getNodeByUUID(String) and the internal SessionImpl.getNodeById(NodeId) methods that
should cover all use cases. If someone wants, I can restore the method signature with a deprecation
mark.
> 
> > Merge UUID to NodeId
> > --------------------
> >
> >                 Key: JCR-1232
> >                 URL: https://issues.apache.org/jira/browse/JCR-1232
> >             Project: Jackrabbit
> >          Issue Type: Improvement
> >          Components: jackrabbit-core
> >            Reporter: Jukka Zitting
> >            Priority: Minor
> >         Attachments: nodeid.patch
> >
> >
> > The current NodeId class is mostly just a wrapper around UUID, which causes two
objects to be instantiated for each node identifier that the system uses. The memory and processing
overhead is quite small, but given that there are tons of NodeId instances it would be good
to eliminate that overhead.
> > There is also lots of code that just converts UUIDs to NodeIds and vice versa. We
could simplify such code if we just used NodeId everywhere.
> > Also, we might want to open up the possibility of using non-UUID node identifiers
at some point in future, so it would make a lot of sense to remove the NodeId.getUUID method
and rely directly on NodeId and it's equals(), hashCode(), and toString() methods in many
places where we currently use UUIDs.
> 


Mime
View raw message