Using a simpler direct "jump to address" instead of "jump register" instructions in MacroAssembler::replaceWithJump we could generate smaller code and we don't need additional nops in branchTrue() and branchFalse(). For this we need to ensure that the generated JIT code is within a 256MB-aligned region. This problem is discussed in https://bugs.webkit.org/show_bug.cgi?id=103146
Created attachment 219523 [details] proposed patch. With using direct jump instruction the code is simpler, the generated code is smaller and the speed improvements are: SunSpider : 5294.0ms -> 5098.7ms v8-v6: 18846.3ms -> 17286.7ms
+ } else if (isWithin256MB(reinterpret_cast<int8_t*>(result) + bytes, bytes)) { + // 2nd half is good, release 1st half. + if (munmap(result, bytes)) + CRASH(); + } else I think the code is wrong here. As we are only returning 2nd half of our result, result should be changed as result + bytes. (i.e, result = reinterpret_cast<int8_t*>(result) + bytes, bytes);) Please comment me, if I'm wrong.
(In reply to comment #2) > I think the code is wrong here. > As we are only returning 2nd half of our result, result should be changed as > result + bytes. (i.e, result = reinterpret_cast<int8_t*>(result) + bytes, > bytes);) > You're right, in this case result should be updated :)
(In reply to comment #3) > (In reply to comment #2) > > I think the code is wrong here. > > As we are only returning 2nd half of our result, result should be changed as > > result + bytes. (i.e, result = reinterpret_cast<int8_t*>(result) + bytes, > > bytes);) > > > > You're right, in this case result should be updated :) I would like to know, how this problem was not observed by any1 else? Because it is very common get an address at the end of 256MB range, isn't it?
> I would like to know, how this problem was not observed by any1 else? Mainly because this change has not been committed in WebKit yet ;) > Because it is very common get an address at the end of 256MB range, isn't it? This case is quite unlikely: the chances that the first mmap doesn't fit between two 256Mb boundardies are low, then the chances that the 2nd mmap doesn't fit are also low. Finally, 50% chance that you'll need the 2nd mmaped area. But of course it's not impossible, so this would need to be fixed.
Comment on attachment 219523 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=219523&action=review We should remove MIPS from svn. You should maintain it downstream. This port doesn't have the kind of regular maintenance that we would expect from an in-tree port. > Source/WTF/wtf/OSAllocatorPosix.cpp:75 > + if (executable && UNLIKELY(!isWithin256MB(result, bytes))) { I don't see how this can happen if you're using a fixed pool.
Comment on attachment 219523 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=219523&action=review >> Source/WTF/wtf/OSAllocatorPosix.cpp:75 >> + if (executable && UNLIKELY(!isWithin256MB(result, bytes))) { > > I don't see how this can happen if you're using a fixed pool. It can happen because of the fixed pool allocation itself: this fixed pool must fit in a 256Mb page.